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

Re: [PATCH v3 05/11] vpci/header: Implement guest BAR register handlers


  • To: Oleksandr Andrushchenko <andr2000@xxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 26 Oct 2021 09:50:46 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=gHC3fXAxMyexqu0p8bQKtwUW1tQhHhpEtL6dn1Etdos=; b=ZLPQ5V4rtJqFtV9JJdZmpk78sacmfvJAs514hNj+WVSVXOuWEpKwKaudM844na2J+Bn7Wry7xxmUzf95sjgCL5xsAftCvzu+UlfyJdTGSW7tmasoyXrqQaV4Hm3atB4WHkPYMivwGg/Rd8p7mHDvNQtQGjhMmovBgmmGniDGIG7mLJHyuqxyHCJtGpnEq4uRI35GCTnyfwYR3H7ElUkk6mvCN5zQSL8doTe7vHK8T17oLDdyc/7MEOL14JUlwsiP5rEC3Cy7Wcb3DbKNxJdwtDxY9rYE9N5z/tTe+oERvYbMjkWMx3J/X4p3VHIt3+FJ1tr7mu3uQAPvBPXyhP5sew==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QvAQvCo8FlZimrMz/X1h11h9a6xN9dEljc5UwChqNglUgAWbV+nYc/7CFs8oi57RB9TZY1Rvo7q7LunjXQKOLjI5ecBvm11wK0pG5K95Wz2p/Q/x2WVJZlxTEXeu51a1XQTZ9o2nmJP4PXHLitTofK+DDr8782QVuJ5CA5b/i/lndP3ssEHvz4fnQLBp+cclUYnBw+el7C1wc1ptIX9yK03Uib5NoakWobeLLTCN4aCnnw12tRgqWQBvEio0FVOkGUhgmaCqIwak4vP4EPy7MTjEhrYxIoKT0szmr9nlU9h988Z6dNZUZrc3BAZrjzeU45RCpOEskoox2yO9vuucaA==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, <julien@xxxxxxx>, <sstabellini@xxxxxxxxxx>, <oleksandr_tyshchenko@xxxxxxxx>, <volodymyr_babchuk@xxxxxxxx>, <Artem_Mygaiev@xxxxxxxx>, <jbeulich@xxxxxxxx>, <bertrand.marquis@xxxxxxx>, <rahul.singh@xxxxxxx>, Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>
  • Delivery-date: Tue, 26 Oct 2021 07:51:06 +0000
  • Ironport-data: A9a23:G3Jb8a3pn/ZNVnXBhfbD5et3kn2cJEfYwER7XKvMYLTBsI5bp2dTz jFOUWqCbKqCYGemeNpzboWy9RwB7MfVyoBkTlQ5pC1hF35El5HIVI+TRqvS04J+DSFhoGZPt Zh2hgzodZhsJpPkS5PE3oHJ9RGQ74nRLlbHILOCan8ZqTNMEn970Es7wbBh2OaEvPDia++zk YKqyyHgEAfNNw5cagr4PIra9XuDFNyr0N8plgRWicJj5TcypFFMZH4rHomjLmOQf2VhNrXSq 9Avbl2O1jixEx8FUrtJm1tgG6EAaua60QOm0hK6V0U+6/TrS+NbPqsTbZIhhUlrZzqhjpNez IR0npqMQCQYZ4fmhb8jSSB1DHQrVUFG0OevzXmXtMWSywvNcmf2wuUoB0YzVWEa0r8pWycUr 6VecW1TKEDY7w616OvTpu1EnMMsIdOtJIoCknph0SvYHbAtRpWrr6DiuIEAg2ds158m8fD2a MgjZGZldUn6PRhCEVkWFZxhsfyuiSyqG9FfgA3M/vdmi4TJ9yR73aLxKtPTdpqPTN9Mg0eDj mvc+iLyBRRyHN6VxCeB83msrvTShi69U4UXfJWR7OJnhWq212MaCRAIfVajqPz/gUm7M/paK kcU8y5oqrIg+UiDR8P4GRa/pRasuRoRWMFZFeEg3wiLxrDJ+AaSBmUCTTlpZcQvsYk9QjlC/ kWAn87tQydutrKVYXuH8/GfqjbaESoIKW4PYwcUQA1D5MPsyKkjgxSKQtt9HaqditzuBSq20 z2MtDI5hbgYkYgMzarT1VnBjyj2/sCRZgEw7wTTGGmi62tReomhYIC57EnB2vxJJo2ZU1qps WANno6V6+VmJZaJmS+WSeMBBoa19u2FOz3Rh11oN5Q5/jHr8HmmFahL+y13LkptNscCeBfqb VXVtAcX44VcVEZGdocuPdj3UZ5zi/G9S5K1DZg4c+aifLBgNyCDrRhFY3KZ/FG0umIxyp8lA LqEJJPE4WkhNYxryz+/RuE42LAtxzwjyW67ea0X3yhLwpLFOybLEeZt3E+mK7ljtvvd8Vq9H 8N3bpPSk31ivPvCjj47GGL5BWsBKmQnHtjIoshTe//rzuFOSTx5VaG5LV/MfeVYc0VpegXgo i7VtqxwkgOXaZj7xeKiMSgLVV8XdcwjxU/XxAR1VbpS51AtYJy08IAUfIYtcL8s+YRLlKAvE qBVK5zRU6QeG1wrHgjxirGm9ORfmOmD31rSb0JJnhBmJ/aMuDAlCve7J1CypUHi/wK8tNcko q3I6+8oacFreuiWN+6PMKjH5wro5RA1wbsuN2OVcog7UBi9q+BCdn2u5sLb1ulRcH0vMBPBj F3IafrZzMGQy7IIHC7h3/DY897ySLsgRSK33QDztN6LCMUTxUL6qadoW+eUZzHNEmTy/aSpf +JOyP/gdvYAmT53X0BUSt6HFIoyuInioaF01ANhECmZZlinEOo4cHKHwdNOputGwboA4Vm6X UeG+997P7SVOZy6TA5NdVR9NunTh+sJnjTy7OguJBmo7iFA47faA15ZOAOBiXIBIeItYp8l2 +oopOUf9xe71kgxKt+Dgy0NrzaMI3UMXr8JrJYfBIO32AMnxksbOc7XCzPs4YHJYNJJaxF4L jiRjavEprJd2kudLCZjSSmThbJQ3M1ctgpLwVkOI0WytuDE3vJnjgdM9TkXTxhOykkV2exEJ WU2ZVZ+Ir+D/mk0iZEbDXysAQxIGDaQ5lf1lwkSjGTcQkSlCj7NIWk6Nbrf9UwV6TsBLD1S/ bXew2f5Szf6Osr223JqC0JirvXiS/139xHDx5/7T5jUQcFibGq3mLKqaEoJtwDjUJE4i0Dwr OV3+Pp9NP/gPikKrqxnU4SX2Nz8kvxfyLCulR25wJ40IA==
  • Ironport-hdrordr: A9a23:39iHaKGa/mjUObWLpLqFDJHXdLJyesId70hD6qkvc3Nom52j+/ xGws536faVslcssHFJo6HmBEClewKnyXcV2/hrAV7GZmfbUQSTXeNfBOfZsljd8mjFh5NgPM RbAtZD4b/LfCFHZK/BiWHSebZQo6j3zEnrv5an854Ed3AUV0gK1XYeNu/0KDwTeOEQbqBJaK Z0q/A37AaISDAyVICWF3MFV+/Mq5nik4/nWwcPA1oC5BOVhT2lxbbmG1zAty1uHg9n8PMHyy zoggb57qKsv7WSzQLd7Xba69BzlMH6wtVOKcSQgow+KynqiCyveIN9Mofy9gwdkaWK0hIHgd PMqxAvM4Ba7G7QRHi8pV/X1wzpwF8Vmgnf4G7dpUGmjd3yRTo8BcYEr5leaAHl500pu8w5+L 5X3kqC3qAnQy/orWDY3ZzlRhtqnk27rT4JiugIlUFSVoMYdft4sZEfxkVIC50NdRiKp7zPKN MeTP002cwmMm9zNxvizytSKZ2XLzgO9y69Mwk/Upf/6UkSoJh7p3Fos/D30E1wsK7VcKM0lN gsBJ4Y4I2mfvVmHZ6VO91xM/dfKla9CC4kY1jiaWgOKsk8SgfwQtjMkfII2N0=
  • Ironport-sdr: WLYFi2Y4xNYx/oAemPMAM1t8EbxW5+y7Ol7bl8p6Mkc+1r69GEnc0pREjv2d2UenPWI7/+v7po xUqwfVrilSefHw9Rc+Tnmgix2AbSenEBJ12KxXru4bCptFvtpa4QHgaLjrLsg4F6Bc4V5/+EOY /TbiVV4evJlsfw8LiylnUuowESo1CxrNHkEocBdjakY8qoaL+bG0SNd3mdppJN09w7Ke0Ng1Hv 6qohqUt340xc6UDl+i4QztXoLmosRENZ1RmZbYs6y+zxnoeIB54o+1HjggaleID7aEhT5YPN01 /R3eksUus3KW4FptII1asyxr
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Thu, Sep 30, 2021 at 10:52:17AM +0300, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> Emulate guest BAR register values: this allows creating a guest view
> of the registers and emulates size and properties probe as it is done
> during PCI device enumeration by the guest.
> 
> ROM BAR is only handled for the hardware domain and for guest domains
> there is a stub: at the moment PCI expansion ROM is x86 only, so it
> might not be used by other architectures without emulating x86. Other
> use-cases may include using that expansion ROM before Xen boots, hence
> no emulation is needed in Xen itself. Or when a guest wants to use the
> ROM code which seems to be rare.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
> Since v1:
>  - re-work guest read/write to be much simpler and do more work on write
>    than read which is expected to be called more frequently
>  - removed one too obvious comment
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> ---
>  xen/drivers/vpci/header.c | 30 +++++++++++++++++++++++++++++-
>  xen/include/xen/vpci.h    |  3 +++
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index 1ce98795fcca..ec4d215f36ff 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -400,12 +400,38 @@ static void bar_write(const struct pci_dev *pdev, 
> unsigned int reg,
>  static void guest_bar_write(const struct pci_dev *pdev, unsigned int reg,
>                              uint32_t val, void *data)
>  {
> +    struct vpci_bar *bar = data;
> +    bool hi = false;
> +
> +    if ( bar->type == VPCI_BAR_MEM64_HI )
> +    {
> +        ASSERT(reg > PCI_BASE_ADDRESS_0);
> +        bar--;
> +        hi = true;
> +    }
> +    else
> +    {
> +        val &= PCI_BASE_ADDRESS_MEM_MASK;
> +        val |= bar->type == VPCI_BAR_MEM32 ? PCI_BASE_ADDRESS_MEM_TYPE_32
> +                                           : PCI_BASE_ADDRESS_MEM_TYPE_64;
> +        val |= bar->prefetchable ? PCI_BASE_ADDRESS_MEM_PREFETCH : 0;
> +    }
> +
> +    bar->guest_addr &= ~(0xffffffffull << (hi ? 32 : 0));
> +    bar->guest_addr |= (uint64_t)val << (hi ? 32 : 0);
> +
> +    bar->guest_addr &= ~(bar->size - 1) | ~PCI_BASE_ADDRESS_MEM_MASK;
>  }
>  
>  static uint32_t guest_bar_read(const struct pci_dev *pdev, unsigned int reg,
>                                 void *data)
>  {
> -    return 0xffffffff;
> +    const struct vpci_bar *bar = data;
> +
> +    if ( bar->type == VPCI_BAR_MEM64_HI )
> +        return bar->guest_addr >> 32;
> +
> +    return bar->guest_addr;

I think this is missing a check for whether the BAR is the high part
of a 64bit one? Ie:

struct vpci_bar *bar = data;
bool hi = false;

if ( bar->type == VPCI_BAR_MEM64_HI )
{
    ASSERT(reg > PCI_BASE_ADDRESS_0);
    bar--;
    hi = true;
}

return bar->guest_addr >> (hi ? 32 : 0);

Or else when accessing the high part of a 64bit BAR you will always
return 0s as it hasn't been setup by guest_bar_write.

Thanks, Roger.



 


Rackspace

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