[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jason Andryuk <jandryuk@xxxxxxxxx>, Paul Durrant <Paul.Durrant@xxxxxxxxxx>
  • From: Simon Gaiser <simon@xxxxxxxxxxxxxxxxxxxxxx>
  • Date: Thu, 14 Mar 2019 20:22:49 +0100
  • Autocrypt: addr=simon@xxxxxxxxxxxxxxxxxxxxxx; prefer-encrypt=mutual; keydata= mQINBFoNGgoBEADpL5fWdBgpH67IYAGPQl0jSevtBzQnjgbiNXu72FfG1Emji2l+mXmHlzxL LnRH+9OyDaOR7tUCbg+iqdbcTowDeFOYUaKQ1+Ub6QiZoi/xZyYlWEGbEc+7grLv1h8ZCt05 Jcp8F73p+ZQhmbT04hhxs4fQ5lsU8T9WJsG4GzF2RKGUbdAk+v/vgu7RFP2bCpqGCv5tAv1x b0geXt4GGNtVqQowDG+1kTnYPousAY2RlexBAjxg5FqM1bliLak8TcoswF7PUhOf8XvqyhI7 H4oySBHUDIyYsreHW64LNqWRK5pFTfqaWssxUyTF8U8Ms8QVNFGyRUWA+erwiv36y8c2McN4 RD7UIZu1x98MbyjqhUcNJj8sNyekDCsClBwKp40aOVjIdvcFZO74M+/SY+5/EUckc1G0+95O J7ZmHU5uEIyovrQvQR9Pypr6NB63OHNXwdfxTPDCnR3vC3r5yNv1+rvrIujO03yWatC5WOGt gGMTwJcbH3tMr354F21p3v/1do3pP88ox279OnbZifnyRyvT6aTo9dFROcG+w4+E4D2sQpwZ lYGbq3Y9f9eQRnBxT/exli8EAcpe5vU7xOjtBFADXN0UDZZajOaNGHtA5o+sFoL6BhKJjuob dNlXtw1zaTJJzCU8k3pZ7xwLsQEVF71KqTN6aGrioG/8S0ZbqwARAQABtDtTaW1vbiBHYWlz ZXIgKElUTCBlbWFpbCBrZXkpIDxzaW1vbkBpbnZpc2libGV0aGluZ3NsYWIuY29tPokCSQQT AQoAMxYhBELmYkepYOm8Nih8D/4aF85p0NysBQJaDRoKAhsBBAsJCAcEFQoJCAIWAAIeAQIX gAAKCRD+GhfOadDcrE8VD/9IPOZcZjPTZkFs7eiqFocr7Ueu/rjy9mJvuZRtQCK2vexNd97z SrPLyKibv/CgpI3Oki2wGrKizgQW4uGDSBd+DDCLaxGmR1PEwPFbwzjEhST1ci/W8IsEsOjG xLe1hcv2U1mtR/WAvjrZcKUs1TGimXuK2u/8OBp+m0xKMUsy30AmyINPtB9LJQSOGqJBOnBc LrK8fGLAIrcVIQyoYiZQKXRFYx4SMCAgOy3Fqh/J2FZfyJXH94HjIXYeOT0t11jD/6vxh5k0 UGP/bDmAWpnHK328BytPlZnf++N+FPfUDyTR1WzFqxHKj5J0YTV0JTA9v0MnvqI71xpb6U9q rZy/yw0FRAe8TJlgoqDbgEDF7+9xM3yJc+1QP4OFs8T65c3xOXTysbRQUzdFnUicSrwj8www QDBlttl1uJnvrHwArJ74pGe2abs8635ABX8SPVxAnUD242Kl71E+C9aCP+XFBPF9rsVPi3YX dq5JzHAOzWHCpCRVelqpkR0ykXm7b6jr+dJ9I0Gs9rOlKisa+GPmyQUzd7XGUO/GW3W8U0pj FUTcMJ+jpjb/UFVpiUIbds0w6emDGZsY9ra/iQSAzFZi9QJR+a88tdeySC1GA2I+UZSbwaKx pqgcXsacRyjR9ph8mGriCU4BMEqoli16pvS4kolMOp9S8KQnh02qX6+7z7kCDQRaDRpEARAA zVDRdV7XDo3M5nGVJYBFqC+vwpRwYOb0BU2IeCuKlH3no9jYSkxPSokN3GEn6bohxvK+nyvS 7b99i2byiGvmg4oRFOUAesHgVAxKpCP9Eerov67Coh/DDBM4hV2d+ZEttQsMQVOnHDLyQGCN 2oVKhenjb53IEaZi0MeyU+wzWhZExGxFAzRL9vJ8X+u1Nw7f+zlWncWEandVZxY6zjmaav21 yJtAvMEuFI+PPYxPMQzKn6gP6XPwf/9chE2VRmguBaLoG0JRNKJ5NSLfYMu9C0A52enWAXJy TwzQ/83DA7W5JXnCjyw6HHA6h3FNW81ASljxHmsOYtcb8J7hAPiFp5Z765jL2loUvbNTncwB mRceX7eQRYNcp6VblvPY68Du5sKx0reHk+ZYmcMa35KBMd3GuXL8dr/onpQpXWiSWyBK0aEC HUippHMiabjeMZ8oTYfq+sXi4jTcQ5h0ThOPG2RMtswlY67xJi1wExIJfp9AflCklUs73Uqg vRksaut+SDGmrYNJBUUkENVvijt1iHJJZJlbWN99bG7KF6pPbj7p+ZHSBTUj53skzDjWdgq1 xuaIv5woZtaRpdggNT/ah4jOEwgLaDtNjsffjIlM/l2+26xxo9sbo8PboZYmrI4vcSO+IXKY 6e7rAz/PTJF/3ITH45sQNQrpYrvRN2dalnkAEQEAAYkEbAQYAQoAIBYhBELmYkepYOm8Nih8 D/4aF85p0NysBQJaDRpEAhsCAkAJEP4aF85p0NyswXQgBBkBCgAdFiEE3E8ezGzG3N1CTQ// kO9xfO/xly8FAloNGkQACgkQkO9xfO/xly9PMw//U+Nu/7rcJ4X3ed7h7AGpAEKWTecs9OWJ IfjzJoE3w2ZAUvhEByT5kXG0cbT44/FGVdp54kQtH2g/ETpIXEfkYGJisEQAwlU93D74wZa5 +5u8RnbyaOyR7bNFOEOngbtyKZohZlJ3AdA++YRunrNUO3TATgGlTgVSlmg9f1+pURcod491 dTDbOIUibQphrTYefdKXt+l5Icx6FfyRDqFgBsm8hRr5fj5h2dGGXvB5Rs/Q/Mhy2ma2a0pY 7w95t6KO7ZWOQsHOUk7eoJKvU04yV1MPDZic+b4VUb2vIZ/qze2eLShf3LoH/4KsNdMPQ0kv 3OY8cjYhHXNyyLo+BWDJ3yFn8fjt6SViE217GfAM5HvV51hQUZEUBrkRSKLTB4yeA/zMNDCR wJRUDFLCZuQfWU0hmQJ2appqmlikgJDBvUVgBUg1GzTI0vGxs1X1Go01fBvIXk2nPLMoVrqs vV6iJPAVZQ2MWAM5koM58APwUG4nj5XKWcMuz31V3IqUKLZ7XE3uqTPcLUPbr53o64/xeQ5z vL5d7uzLkyhQKNH3h7LysG9gi7nj3cAfMlbX/ameBTOsP7SE8j4bJA6BmpQ4UF1tr8uO7xDy cpLWbfV4ocQrN12xVnaAlGNWI4bQfAleCJEn7F96THWQSslA+u/rTvI/IizKCLuu2+gnispT uajXPxAAvwXraqyw/mlQe+sEJip+ivgJ5jOtT+RY2z5fTja9T9k/vADcuXowJN0TP3w82rz6 JG0LSD1OGiRUTqk8TYXYdOdkBU7rFI5MMoi7SbtF2S9B7urqTRbAOYrqMu2jzzhFonJ0yGFS kelooiRf77gFqJx5IuBGPVTzPkr/YhhrgNDo79sYGusnaae/5VjpF2JWiN338jBtxbieNRpb zVf31PbeNCqAcc324RXSZ5AtxEGszyC9I/OMonCTnXAX02TbyN8hcNIAfWBkV4lmFI+LKo8v PwxIjbza1b9KHybp4y1pIunG+xhPdxARmeyLUM7JTAgap3gHG3kCdiDpiVLCcUEkrrmRxshI sToCsQHJyLwp87ECSc9HXKMLETSiyzbn4QJkIt7DnZR5kAg4PQBEnWeL7dys4TYO6ymslFaw RkowIqcDXU6cO1k9qwOR1nBv1GqLovoixbKWmms+uDSg4ZauZRzxCemmWhGurxJEso7t8AjI ZVXbcAaJFV+eNlWFLpMRlEpGfma0f6Jna9N28cxHoFC53+0EdS2C2XSPWkuR30zxJ02196S/ zaO3hu9J8ZTs7i+vZMkcMzWQJ473O3veyZ6de+LOq5qwOMGLO6RGCghzqYGwPBO1JJLwMmQm 1qOGiz/FBQ2YLgyTh3ukxZe4ilJ8OGW1XrnxoXDaZAm5Ag0EWg0ahgEQANPvpcDGYMFdkz5W 1sb/c2a6ydelDD6fkq3dWdG7ylgYp7qfApuPqVY8e0WLx3BqqcRSJ5zgnFMPhnicXCJIm4Ag UJnKn6uZplCR+ewbDf7EcnF3l08iiHxCAsOzyVoMOByMMF24x4ZGffMtMoATnic7mGWvAaEp wKVGIeQWslY+cYRfedMkxytwD96nIUsh7MF16KkcJYes6DeZ1p1e/1UOuArHGMJ7aRT77UUJ r7cf/GVwPYLfBjVt9j/5Ft/PIfsjn3A8WjGKQ+wSA03ck63jea1WLGXUFXeGNm3kQX7q/Kz2 h+TUvTYaI4mQUj2W3v2DX/6dFlC3qX3gV/pg0i5MG9J//mYfcf/svIoP/IHf6jlj8QMguPih FdZSO5oj5Ms0UX4q5wcvl3sF4DtZJq6DathJj1laUMOxkX0ZUy7dA0HO6ggJ9qp2qxIb3SlY Y3KdXw9kM9/PkGFXAbY/gqwWSoYc4PEE0aIDro+OYMMjoVLCD0P8ocKMb0fpnJR8jKVZpVqx yA0cXpnkcOGhU6gjCvrOJRXAaZYvG+4KZyOTkk7C2rqH9eenafhRe8xYsrEmKPjKdH1GJtab M+4+zZxKytkX8SNodS5pUXCGLvj/aFET/boDgHyVVCSWAJyMVVWNiZvjETh614zjz1milOXJ S6hFnHiqi5Y08r0xDVmHABEBAAGJAjYEGAEKACACGwwWIQRC5mJHqWDpvDYofA/+GhfOadDc rAUCWg0b1QAKCRD+GhfOadDcrIH6EACoGIhemgY9hHBnUagcfCSA92DvtJSm5ZjpKrZyKuGh TVWNjx+H4A++/w6pDFZHo0LCtLvJikA2dpeowBf55Lts+mDLOCSikxjTrdaV7kjtbay5MmnS ytu4GNBa84UShfX1N2JPr/b4+O/dNo1fsby7iI7tDV431PMymFMXUNrsexqPU+u24jpPNFKh /r1La7FMQn0djusE5Uvq5O6T4jFMfsNgbmHWuFME8tQBywNSgcY4SE26+O0/xGSvUCAZ1mFA 52DNtMxm+hAxwrrzKZWhX7Hx9XCsGaOWEV9iwI1pfY4xRibXogtEQShifAYVwZYOGq+fo2ka MefIkujf8o0P8UQAqk2xHCAQ6NCBRqHku7Q1Rk05GlrI2XC8zSSWoa88eha/De+ePZbdRTW9 1tdUyFN15S+r3e2+uDl4a6C068e2+gv0dDeFhmVW+ZIpdShmsGxBVMCCFz9tAwyArvSVRBjA aA5MHXGiC/IU4fuBjt0hHVQ1tbS9Ib7UFe1HryRVtzpf5ljAgEbP1aRmKIone1X0HOG17Wr4 P9grm7+TVE7aSs3zRsB5Y11pDTcRKfWpXyuiWi0rsZXyO25JkKUw4HSjaL3EttfyLBGl6K0H Q2WwHZtayb8owb/ZKWlTr6cgwY88KC1RP+kdc63zhKcJRmbeMPrvHLygk+IoG2AYbg==
  • Cc: Anthony Perard <anthony.perard@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, "qemu-devel@xxxxxxxxxx" <qemu-devel@xxxxxxxxxx>, "marmarek@xxxxxxxxxxxxxxxxxxxxxx" <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 14 Mar 2019 19:23:13 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

Jason Andryuk:
> 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?

Not sure which one it was. Probably the USB controller or the SD host
controller. As your example below shows this is not so uncommon.

> 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.
> 
> Regards,
> Jason
> 
>>> 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@xxxxxxxxxxxxxxxxxxxxxx>
>>> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
>>> ---
>>>  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

Attachment: signature.asc
Description: OpenPGP digital signature

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.