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

Re: [PATCH v2 1/3] xen/vpci: msix: move x86 specific code to x86 file


  • To: Rahul Singh <Rahul.Singh@xxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 9 Mar 2022 12:17:42 +0100
  • 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=+Nah9IVu+bsiNX23p3/K8fCHUk1qdH8r7XMO3YfzH5M=; b=Yhuq8qGZdDzjxJC9/BHeK+m/G+QB0JF4KnX/oykHOMeBXdgwx9UF41mR+amKPNagcHEntfeAhiEgoruXbxYw6g7yriPiAng/C4RLCFNJ3Nv6+FFeYdaUtjOpzPYmBuNe7Yo8zbtE3OHOL1irwy/0+xLCZXadngXZbpyYGbjHdHttVbJP5vjPxt7CH6L7+ils+L69XE3X6htODrEg9PolnbxlM4Gm/3jOIl0tXsIyUzX+H7YcXxhkJ5uTDLGr8h1sL/YV+jgQgk9rkIGVahy11HP53ZXBA+QhkKM5J1AtwzhqmJwrp02wrKDvXG1dmpFpClKcHLmcFqTaPi5/+kUv5g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Q0GYg9NMBiC3VpSzni7hyFMiynsLV9JxYSfi7SHkFlotme9WOhTpN++3P/jUs0g9oaRBpBxlYGnZiOosh5Owx0hva1y7/5RBaQvapL3+qzdQcTqG6cWovImIkYMHRaIE0ixyZ39LwTKBD/037P5faTKIF72jlyX761rQKdaKDIvY1Dv0ubjSCCDNGtRNx8NbC2j42niU9MWJ3O/zgzDFBBdSeS+n84lT1rOJd51765LjLzckkzEt92/sy872W14z2GlecJM7FIvfKL+z+vXsRiReICSXV0j/X3lCxtTxjAKIfZsRiE3hh5gbiyhYGrBVHfqk8KtTtt4wBcsZSZppRA==
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Jan Beulich <jbeulich@xxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Wed, 09 Mar 2022 11:18:11 +0000
  • Ironport-data: A9a23:l/M6nq0PkV2BDTIIv/bD5S9xkn2cJEfYwER7XKvMYLTBsI5bpzAHy GFNC2GHP6rbNGfwLYpyYNmw/ElUu5XUyoVrHlBqpC1hF35El5HIVI+TRqvS04J+DSFhoGZPt Zh2hgzodZhsJpPkjk7xdOCn9xGQ7InQLlbGILes1htZGEk1EE/NtTo5w7Rj2tUw0IDja++wk YiaT/P3aQfNNwFcagr424rbwP+4lK2v0N+wlgVWicFj5DcypVFMZH4sDfjZw0/DaptVBoaHq 9Prl9lVyI97EyAFUbtJmp6jGqEDryW70QKm0hK6UID66vROS7BbPg/W+5PwZG8O4whlkeydx /1qtr2WSDkvO5fwt+BaSDNIShNAZYNJreqvzXiX6aR/zmXDenrohf5vEFs3LcsT/eMf7WNmr KJCbmpXN1ba2rzwkOnTpupE36zPKOHxO4wSoDd4xCzxBvc6W5HTBa7N4Le02R9u25AWRKuHO qL1bxJRfDnNZixLOWxIN8MUvsmRjV38YwJx/Qf9Sa0fvDGIkV0ZPKLWGMHOZtWASMFRn0CZj mHL5WL0BlcdLtP34TiY9nOhgMffkCW9X5gdfJWg/+NuqE2ewCoUEhJ+fUu2p7y1h1CzX/pbK lcI4Ww+oK4q7kupQ9LhGRqirxasvBQRRt5RGO0S8xyWx+zf5APxLlINTiNFLucnssA2bTUw0 xmCmNaBLSNrmK2YTzSa7Lj8hRm/IzQPJGkOIwoNVxIY4sLLqZs2yBnIS75e/LWd14OvX2uqm nbT8XZ41+57YdM3O7uT2lPmpTOSoJ3zciExoR79TjuX0FtXa9vwD2C30mTz4fFFJYefa1COu nkYhsSThNwz4YGxeD+lG7tUQuzwjxqRGHiF2AM0QcF9n9i40yP7JehtDCdCyFCF2yruURvge wfttAxY//e/11P6PPYsM+pd5ynHpJUM9OgJtNiJNrKigbArLWdrGR2Cg2bKgQgBd2B2zckC1 W+zK5rEMJrjIf0PIMCKb+kcy6Q34Ss12HneQ5v2pzz+j+bAOyDJE+dZbAfQBgzc0E9iiF+Fm zq4H5HWoyizrcWkOnWHmWLtBQtiwYcH6WDe9JUMK7/rzvtOE2A9Ef7BqY7NiKQ+95m5Ytzgp ynnMmcBkQKXrSSedW2iNyAyAJuyDM0XhS9qYkQR0aOAhiFLjXCHt/xEKfPavNAPqYRe8BKDZ 6JcKpvaXagXFGivFvZ0RcCVkbGOvS+D3GqmFyGkfCI+b9hnQQnI8cXjZQzh6G8FCS/fiCf0i +f5vu8HafLvnzhfMfs=
  • Ironport-hdrordr: A9a23:rtjoBK1nGvvEiKgS7OqXdwqjBVByeYIsimQD101hICG9Lfb2qy n+ppgmPEHP5Qr5OEtApTiBUJPwJk800aQFm7X5Wo3SITUO2VHYV72KiLGN/9SOIVydygcw79 YET0E6MqyNMbEYt7eK3ODbKadY/DDvysnB7o2/vhRQpENRGtldBm9Ce3im+yZNNW977PQCZf 6hDp0tnUveRZ1bVLXyOlA1G8z44/HbnpPvZhALQzYh9Qm1lDutrJr3CQKR0BsyWy5Ghe5Kyx mOryXJooGY992rwB7V0GHeq7xQhdva09NGQOiBkNIcJDnAghuhIK5hR7qBljYop/zH0idgrP D85zMbe+hj4XLYeW+45TPrxgnbyT4rr0TvzFeJ6EGT1vDRdXYfMY5slIhZehzW5w4Lp9dnyp 9G2Gqfqt5+EQ7AtD6V3amGazha0m6P5VYym+8aiHJSFaEEbqVKkIAZ9ERJVL8dASPB7pw9Gu UGNrCR2B9vSyLaU5nlhBgu/DT1NU5DXStuA3Jy9/B96gIm0kyQlCAjtY4idnRpzuNId3AL3Z WADk1SrsA8ciYnV9MMOA4/e7rENoXse2O7DIvAGyWvKEk4U0i93qIfpo9FoN2XRA==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Mar 09, 2022 at 10:47:06AM +0000, Rahul Singh wrote:
> Hi Roger,
> 
> > On 9 Mar 2022, at 10:16 am, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> > 
> > On Wed, Mar 09, 2022 at 10:08:12AM +0000, Rahul Singh wrote:
> >> Hi Jan,
> >> 
> >>> On 4 Mar 2022, at 7:23 am, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>> 
> >>> On 03.03.2022 17:31, Rahul Singh wrote:
> >>>>> On 1 Mar 2022, at 1:55 pm, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>>>> On 01.03.2022 14:34, Rahul Singh wrote:
> >>>>>>> On 24 Feb 2022, at 2:57 pm, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>>>>>> On 15.02.2022 16:25, Rahul Singh wrote:
> >>>>>>>> --- a/xen/arch/x86/hvm/vmsi.c
> >>>>>>>> +++ b/xen/arch/x86/hvm/vmsi.c
> >>>>>>>> @@ -925,4 +925,106 @@ int vpci_msix_arch_print(const struct 
> >>>>>>>> vpci_msix *msix)
> >>>>>>>> 
> >>>>>>>>  return 0;
> >>>>>>>> }
> >>>>>>>> +
> >>>>>>>> +int vpci_make_msix_hole(const struct pci_dev *pdev)
> >>>>>>>> +{
> >>>>>>>> +    struct domain *d = pdev->domain;
> >>>>>>>> +    unsigned int i;
> >>>>>>>> +
> >>>>>>>> +    if ( !pdev->vpci->msix )
> >>>>>>>> +        return 0;
> >>>>>>>> +
> >>>>>>>> +    /* Make sure there's a hole for the MSIX table/PBA in the p2m. 
> >>>>>>>> */
> >>>>>>>> +    for ( i = 0; i < ARRAY_SIZE(pdev->vpci->msix->tables); i++ )
> >>>>>>>> +    {
> >>>>>>>> +        unsigned long start = PFN_DOWN(vmsix_table_addr(pdev->vpci, 
> >>>>>>>> i));
> >>>>>>>> +        unsigned long end = PFN_DOWN(vmsix_table_addr(pdev->vpci, 
> >>>>>>>> i) +
> >>>>>>>> +                                     vmsix_table_size(pdev->vpci, 
> >>>>>>>> i) - 1);
> >>>>>>>> +
> >>>>>>>> +        for ( ; start <= end; start++ )
> >>>>>>>> +        {
> >>>>>>>> +            p2m_type_t t;
> >>>>>>>> +            mfn_t mfn = get_gfn_query(d, start, &t);
> >>>>>>>> +
> >>>>>>>> +            switch ( t )
> >>>>>>>> +            {
> >>>>>>>> +            case p2m_mmio_dm:
> >>>>>>>> +            case p2m_invalid:
> >>>>>>>> +                break;
> >>>>>>>> +            case p2m_mmio_direct:
> >>>>>>>> +                if ( mfn_x(mfn) == start )
> >>>>>>>> +                {
> >>>>>>>> +                    clear_identity_p2m_entry(d, start);
> >>>>>>>> +                    break;
> >>>>>>>> +                }
> >>>>>>>> +                /* fallthrough. */
> >>>>>>>> +            default:
> >>>>>>>> +                put_gfn(d, start);
> >>>>>>>> +                gprintk(XENLOG_WARNING,
> >>>>>>>> +                        "%pp: existing mapping (mfn: %" PRI_mfn
> >>>>>>>> +                        "type: %d) at %#lx clobbers MSIX MMIO 
> >>>>>>>> area\n",
> >>>>>>>> +                        &pdev->sbdf, mfn_x(mfn), t, start);
> >>>>>>>> +                return -EEXIST;
> >>>>>>>> +            }
> >>>>>>>> +            put_gfn(d, start);
> >>>>>>>> +        }
> >>>>>>>> +    }
> >>>>>>>> +
> >>>>>>>> +    return 0;
> >>>>>>>> +}
> >>>>>>> 
> >>>>>>> ... nothing in this function looks to be x86-specific, except maybe
> >>>>>>> functions like clear_identity_p2m_entry() may not currently be 
> >>>>>>> available
> >>>>>>> on Arm. But this doesn't make the code x86-specific.
> >>>>>> 
> >>>>>> I will maybe be wrong but what I understand from the code is that for 
> >>>>>> x86 
> >>>>>> if there is no p2m entries setup for the region, accesses to them will 
> >>>>>> be trapped 
> >>>>>> into the hypervisor and can be handled by specific MMIO handler.
> >>>>>> 
> >>>>>> But for ARM when we are registering the MMIO handler we have to 
> >>>>>> provide 
> >>>>>> the GPA also for the MMIO handler. 
> > 
> > Right, but you still need those regions to not be mapped on the second
> > stage translation, or else no trap will be triggered and thus the
> > handlers won't run?
> > 
> > Regardless of whether the way to register the handlers is different on
> > Arm and x86, you still need to assure that the MSI-X related tables
> > are not mapped on the guest second stage translation, or else you are
> > just allowing guest access to the native ones.
> 
> What I understand from the VPCI code we are not mapping the MSI-X related 
> tables/BAR
> to Stage-2 translation therefore no need to remove the mapping.
> 
> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/vpci/header.c;h=a1c928a0d26f5cefd98727ee37f5dc43ceba80a6;hb=HEAD#l248

Right, sorry, was slightly confused. So this is indeed only needed if
Arm does some kind of pre-mapping of non-RAM regions. For example an
x86 PVH dom0 will add the regions marked as 'reserved' to the second
stage translation, and we need vpci_make_msix_hole in order to punch
holes there if those pre-mapped regions happen to overlap with any
MSI-X table.

If there aren't any non-RAM regions mapped on Arm for it's hardware
domain by default then I guess it's safe to make this arch-specific.

Thanks, Roger.



 


Rackspace

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