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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jason Andryuk [mailto:jandryuk@xxxxxxxxx]
> Sent: 14 March 2019 18:16
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: qemu-devel@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx; 
> marmarek@xxxxxxxxxxxxxxxxxxxxxx; Simon
> Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Anthony Perard
> <anthony.perard@xxxxxxxxxx>
> Subject: Re: [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE
> 
> On Wed, Mar 13, 2019 at 11:09 AM Paul Durrant <Paul.Durrant@xxxxxxxxxx> wrote:
> >
> > > -----Original Message-----
> > > From: Jason Andryuk [mailto:jandryuk@xxxxxxxxx]
> > > Sent: 11 March 2019 18:02
> > > To: qemu-devel@xxxxxxxxxx
> > > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; marmarek@xxxxxxxxxxxxxxxxxxxxxx; 
> > > Simon Gaiser
> > > <simon@xxxxxxxxxxxxxxxxxxxxxx>; Jason Andryuk <jandryuk@xxxxxxxxx>; 
> > > Stefano Stabellini
> > > <sstabellini@xxxxxxxxxx>; Anthony Perard <anthony.perard@xxxxxxxxxx>; 
> > > Paul Durrant
> > > <Paul.Durrant@xxxxxxxxxx>
> > > Subject: [PATCH 6/6] xen-pt: Round pci regions sizes to XEN_PAGE_SIZE
> > >
> > > From: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
> > >
> > > 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
> >
> 
> Hi, Paul.
> 
> Simon found this, so it affects a real device.  Simon, do you recall
> which device was affected?
> 
> I think BARs only need to be power-of-two size and aligned, and 4k is
> not a minimum.  16bytes may be a minimum, but I don't know what the
> spec says.
> 
> On an Ivy Bridge system, here are some of the devices with BARs smaller than 
> 4K:
> 00:16.0 Communication controller: Intel Corporation 7 Series/C210
> Series Chipset Family MEI Controller #1 (rev 04)
>    Memory at d0735000 (64-bit, non-prefetchable) [disabled] [size=16]
> 00:1d.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
> Family USB Enhanced Host Controller #1 (rev 04) (prog-if 20 [EHCI])
>    Memory at d0739000 (32-bit, non-prefetchable) [disabled] [size=1K]
> 00:1f.3 SMBus: Intel Corporation 7 Series/C210 Series Chipset Family
> SMBus Controller (rev 04)
>    Memory at d0734000 (64-bit, non-prefetchable) [disabled] [size=256]
> 02:00.0 System peripheral: JMicron Technology Corp. SD/MMC Host
> Controller (rev 30)
>    Memory at d0503000 (32-bit, non-prefetchable) [disabled] [size=256]
> 
> These examples are all 4K aligned, so this is not an issue on this machine.
> 
> Reviewing the code, I'm now wondering if the following in
> hw/xen/xen_pt.c:xen_pt_region_update is wrong:        rc =
> xc_domain_memory_mapping(xen_xc, xen_domid,
>                                      XEN_PFN(guest_addr + XC_PAGE_SIZE - 1),
>                                      XEN_PFN(machine_addr + XC_PAGE_SIZE - 1),
>                                      XEN_PFN(size + XC_PAGE_SIZE - 1),
>                                      op);
> 
> If a bar of size 0x100 is at 0xd0500800, then the machine_addr passed
> in would be 0xd0501000 which is past the actual location.  Should the
> call arguments just be XEN_PFN(guest_addr) & XEN_PFN(machine_addr)?
> 
> BARs smaller than a page would also be a problem if BARs for different
> devices shared the same page.

Exactly. We cannot pass them through with any degree of safety (not that 
passthrough of an arbitrary device is a particularly safe thing to do anyway). 
The xen-pt code would instead need to trap those BARs and perform the accesses 
to the real BAR itself. Ultimately though I think we should be retiring the 
xen-pt code in favour of a standalone emulator.

  Paul 

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