Re: [Qemu-devel] [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE

[Paul Durrant]

> -----Original Message-----
> From: Jason Andryuk [mailto:address@hidden
> Sent: 11 March 2019 18:02
> To: address@hidden
> Cc: address@hidden; address@hidden; Simon Gaiser
> <address@hidden>; Jason Andryuk <address@hidden>; Stefano Stabellini
> <address@hidden>; Anthony Perard <address@hidden>; Paul Durrant
> <address@hidden>
> Subject: [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE
> 
> From: Simon Gaiser <address@hidden>
> 
> If a pci memory region has a size < XEN_PAGE_SIZE it can get located at
> an address which is not page aligned.

IIRC the PCI spec says that the minimum memory region size should be at least 
4k. Should we even be tolerating BARs smaller than that?

  Paul

> This breaks the memory mapping via
> xc_domain_memory_mapping since this function is page based and the
> "offset" is therefore lost.
> 
> Without this patch you will see error like this in the stubdom log:
> 
>   [00:05.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
> @0x0000000000000004
> 
> QubesOS/qubes-issues#2849
> 
> Signed-off-by: Simon Gaiser <address@hidden>
> Signed-off-by: Jason Andryuk <address@hidden>
> ---
>  hw/xen/xen_pt.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index 5539d56c3a..7f680442ee 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -449,9 +449,10 @@ static int 
> xen_pt_register_regions(XenPCIPassthroughState *s, uint16_t *cmd)
>      /* Register PIO/MMIO BARs */
>      for (i = 0; i < PCI_ROM_SLOT; i++) {
>          XenHostPCIIORegion *r = &d->io_regions[i];
> +        pcibus_t r_size = r->size;
>          uint8_t type;
> 
> -        if (r->base_addr == 0 || r->size == 0) {
> +        if (r->base_addr == 0 || r_size == 0) {
>              continue;
>          }
> 
> @@ -469,15 +470,18 @@ static int 
> xen_pt_register_regions(XenPCIPassthroughState *s, uint16_t *cmd)
>                  type |= PCI_BASE_ADDRESS_MEM_TYPE_64;
>              }
>              *cmd |= PCI_COMMAND_MEMORY;
> +
> +            /* Round up to a full page for the hypercall. */
> +            r_size = (r_size + XC_PAGE_SIZE - 1) & XC_PAGE_MASK;
>          }
> 
>          memory_region_init_io(&s->bar[i], OBJECT(s), &ops, &s->dev,
> -                              "xen-pci-pt-bar", r->size);
> +                              "xen-pci-pt-bar", r_size);
>          pci_register_bar(&s->dev, i, type, &s->bar[i]);
> 
>          XEN_PT_LOG(&s->dev, "IO region %i registered (size=0x%08"PRIx64
>                     " base_addr=0x%08"PRIx64" type: %#x)\n",
> -                   i, r->size, r->base_addr, type);
> +                   i, r_size, r->base_addr, type);
>      }
> 
>      /* Register expansion ROM address */
> --
> 2.20.1