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

From: Paul Durrant <Paul.Durrant@citrix.com>
To: 'Jason Andryuk' <jandryuk@gmail.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: Simon Gaiser <simon@invisiblethingslab.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Stefano Stabellini <sstabellini@kernel.org>,
	"marmarek@invisiblethingslab.com"
	<marmarek@invisiblethingslab.com>,
	Anthony Perard <anthony.perard@citrix.com>
Subject: Re: [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE
Date: Wed, 13 Mar 2019 15:09:33 +0000	[thread overview]
Message-ID: <ee0da83d3f054e72ae450437c8834d04@AMSPEX02CL02.citrite.net> (raw)
In-Reply-To: <20190311180216.18811-7-jandryuk@gmail.com>

> -----Original Message-----
> From: Jason Andryuk [mailto:jandryuk@gmail.com]
> Sent: 11 March 2019 18:02
> To: qemu-devel@nongnu.org
> Cc: xen-devel@lists.xenproject.org; marmarek@invisiblethingslab.com; Simon Gaiser
> <simon@invisiblethingslab.com>; Jason Andryuk <jandryuk@gmail.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Anthony Perard <anthony.perard@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>
> Subject: [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE
> 
> From: Simon Gaiser <simon@invisiblethingslab.com>
> 
> 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 <simon@invisiblethingslab.com>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> ---
>  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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel