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

Re: [PATCH v2 1/4] IOMMU/x86: switch to alternatives-call patching in further instances


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Rahul Singh <Rahul.Singh@xxxxxxx>
  • Date: Fri, 28 Jan 2022 10:40:33 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; 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=ZZX1gX9T+0+Pvj6y9Aebg33B2F67hCTSSTW9AtwOm+A=; b=a++8/yhdF4CfBf/wxiQZmuombL+wwbJkNb2fdRpZ6FmkTGdEd3eFq0rfXAMAhh3UFUxivgVCIhR4Xc41doaVIJPqfx2Srmn1Qqh1Yt4aAdnm0HqC0hffEBP/fOJuMhQKOGCG/E8gFoGgPx54zStX7fqUkulLf4CFM34UEwKbrQGXIfDH4CyryYZHfcWv+/7dY5Ik4CzXo5ny4qyklFsWTSeGw3BEf8A+Q8To3j6NA0xK38Jy4lcMJv0BS/IEPgHTFEuJWXt7g0Q0pgbN4Nx2ucJunAkjPsplQvF8BEjpKd/TRP8fdzXtjzbsaB/6WBiPbs/B2ufsCLyhL+CLLG4YAg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TGQV2Sn381EzPEagpSlu6YEUEB4n90l/1mkouO90VhdMsYkIpylPLDgwrqEUwk/5t86/bhDeyFdpOFJ3NkzYzgoWvr7UQGb1oAZis8nkAbNy/sfEVYdmnX9dLRzXoQaGqQynVx2uxKlb3D0WHFEghyT/Y1p84tZDnwMLegyabaLIhvZZKYbE7FVOAxczdp5Yste/KY6qqwHKY4tXrzZT1pb/YYe73TTgg0NScqjCQ6qP+JxaTwHzZZHmsiX3OawCoQFrKfaZkEW55aci5VGYcRJs352aDDSUav/aVBE7jTxfyJ2FRwbzxlW3ymWA3s4ivnaXFntI17MhcBRDO/wOYA==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Fri, 28 Jan 2022 10:40:51 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHYE4zovlZrTiTMtEaG1m4PWdsJMqx4P9gA
  • Thread-topic: [PATCH v2 1/4] IOMMU/x86: switch to alternatives-call patching in further instances

Hi Jan,

> On 27 Jan 2022, at 2:47 pm, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> This is, once again, to limit the number of indirect calls as much as
> possible. The only hook invocation which isn't sensible to convert is
> setup(). And of course Arm-only use sites are left alone as well.
> 
> Note regarding the introduction / use of local variables in pci.c:
> struct pci_dev's involved fields are const. This const propagates, via
> typeof(), to the local helper variables in the altcall macros. These
> helper variables are, however, used as outputs (and hence can't be
> const). In iommu_get_device_group() make use of the new local variables
> to also simplify some adjacent code.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Rahul Singh <rahul.singh@xxxxxxx>
Tested-by: Rahul Singh <rahul.singh@xxxxxxx>

Regards,
Rahul
> 
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -198,7 +198,7 @@ int iommu_domain_init(struct domain *d,
>         return ret;
> 
>     hd->platform_ops = iommu_get_ops();
> -    ret = hd->platform_ops->init(d);
> +    ret = iommu_call(hd->platform_ops, init, d);
>     if ( ret || is_system_domain(d) )
>         return ret;
> 
> @@ -233,7 +233,7 @@ void __hwdom_init iommu_hwdom_init(struc
> 
>     register_keyhandler('o', &iommu_dump_page_tables, "dump iommu page 
> tables", 0);
> 
> -    hd->platform_ops->hwdom_init(d);
> +    iommu_vcall(hd->platform_ops, hwdom_init, d);
> }
> 
> static void iommu_teardown(struct domain *d)
> @@ -576,7 +576,7 @@ int iommu_get_reserved_device_memory(iom
>     if ( !ops->get_reserved_device_memory )
>         return 0;
> 
> -    return ops->get_reserved_device_memory(func, ctxt);
> +    return iommu_call(ops, get_reserved_device_memory, func, ctxt);
> }
> 
> bool_t iommu_has_feature(struct domain *d, enum iommu_feature feature)
> @@ -603,7 +603,7 @@ static void iommu_dump_page_tables(unsig
>             continue;
>         }
> 
> -        dom_iommu(d)->platform_ops->dump_page_tables(d);
> +        iommu_vcall(dom_iommu(d)->platform_ops, dump_page_tables, d);
>     }
> 
>     rcu_read_unlock(&domlist_read_lock);
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -861,15 +861,15 @@ static int deassign_device(struct domain
>         devfn += pdev->phantom_stride;
>         if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
>             break;
> -        ret = hd->platform_ops->reassign_device(d, target, devfn,
> -                                                pci_to_dev(pdev));
> +        ret = iommu_call(hd->platform_ops, reassign_device, d, target, devfn,
> +                         pci_to_dev(pdev));
>         if ( ret )
>             goto out;
>     }
> 
>     devfn = pdev->devfn;
> -    ret = hd->platform_ops->reassign_device(d, target, devfn,
> -                                            pci_to_dev(pdev));
> +    ret = iommu_call(hd->platform_ops, reassign_device, d, target, devfn,
> +                     pci_to_dev(pdev));
>     if ( ret )
>         goto out;
> 
> @@ -1300,7 +1300,7 @@ static int iommu_add_device(struct pci_d
> {
>     const struct domain_iommu *hd;
>     int rc;
> -    u8 devfn;
> +    unsigned int devfn = pdev->devfn;
> 
>     if ( !pdev->domain )
>         return -EINVAL;
> @@ -1311,16 +1311,16 @@ static int iommu_add_device(struct pci_d
>     if ( !is_iommu_enabled(pdev->domain) )
>         return 0;
> 
> -    rc = hd->platform_ops->add_device(pdev->devfn, pci_to_dev(pdev));
> +    rc = iommu_call(hd->platform_ops, add_device, devfn, pci_to_dev(pdev));
>     if ( rc || !pdev->phantom_stride )
>         return rc;
> 
> -    for ( devfn = pdev->devfn ; ; )
> +    for ( ; ; )
>     {
>         devfn += pdev->phantom_stride;
>         if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
>             return 0;
> -        rc = hd->platform_ops->add_device(devfn, pci_to_dev(pdev));
> +        rc = iommu_call(hd->platform_ops, add_device, devfn, 
> pci_to_dev(pdev));
>         if ( rc )
>             printk(XENLOG_WARNING "IOMMU: add %pp failed (%d)\n",
>                    &pdev->sbdf, rc);
> @@ -1341,7 +1341,7 @@ static int iommu_enable_device(struct pc
>          !hd->platform_ops->enable_device )
>         return 0;
> 
> -    return hd->platform_ops->enable_device(pci_to_dev(pdev));
> +    return iommu_call(hd->platform_ops, enable_device, pci_to_dev(pdev));
> }
> 
> static int iommu_remove_device(struct pci_dev *pdev)
> @@ -1363,7 +1363,8 @@ static int iommu_remove_device(struct pc
>         devfn += pdev->phantom_stride;
>         if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
>             break;
> -        rc = hd->platform_ops->remove_device(devfn, pci_to_dev(pdev));
> +        rc = iommu_call(hd->platform_ops, remove_device, devfn,
> +                        pci_to_dev(pdev));
>         if ( !rc )
>             continue;
> 
> @@ -1371,7 +1372,9 @@ static int iommu_remove_device(struct pc
>         return rc;
>     }
> 
> -    return hd->platform_ops->remove_device(pdev->devfn, pci_to_dev(pdev));
> +    devfn = pdev->devfn;
> +
> +    return iommu_call(hd->platform_ops, remove_device, devfn, 
> pci_to_dev(pdev));
> }
> 
> static int device_assigned(u16 seg, u8 bus, u8 devfn)
> @@ -1421,7 +1424,8 @@ static int assign_device(struct domain *
> 
>     pdev->fault.count = 0;
> 
> -    if ( (rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), 
> flag)) )
> +    if ( (rc = iommu_call(hd->platform_ops, assign_device, d, devfn,
> +                          pci_to_dev(pdev), flag)) )
>         goto done;
> 
>     for ( ; pdev->phantom_stride; rc = 0 )
> @@ -1429,7 +1433,8 @@ static int assign_device(struct domain *
>         devfn += pdev->phantom_stride;
>         if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
>             break;
> -        rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), 
> flag);
> +        rc = iommu_call(hd->platform_ops, assign_device, d, devfn,
> +                        pci_to_dev(pdev), flag);
>     }
> 
>  done:
> @@ -1457,24 +1462,24 @@ static int iommu_get_device_group(
>     if ( !is_iommu_enabled(d) || !ops->get_device_group_id )
>         return 0;
> 
> -    group_id = ops->get_device_group_id(seg, bus, devfn);
> +    group_id = iommu_call(ops, get_device_group_id, seg, bus, devfn);
> 
>     pcidevs_lock();
>     for_each_pdev( d, pdev )
>     {
> -        if ( (pdev->seg != seg) ||
> -             ((pdev->bus == bus) && (pdev->devfn == devfn)) )
> +        unsigned int b = pdev->bus;
> +        unsigned int df = pdev->devfn;
> +
> +        if ( (pdev->seg != seg) || ((b == bus) && (df == devfn)) )
>             continue;
> 
> -        if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (pdev->bus << 8) | 
> pdev->devfn) )
> +        if ( xsm_get_device_group(XSM_HOOK, (seg << 16) | (b << 8) | df) )
>             continue;
> 
> -        sdev_id = ops->get_device_group_id(seg, pdev->bus, pdev->devfn);
> +        sdev_id = iommu_call(ops, get_device_group_id, seg, b, df);
>         if ( (sdev_id == group_id) && (i < max_sdevs) )
>         {
> -            bdf = 0;
> -            bdf |= (pdev->bus & 0xff) << 16;
> -            bdf |= (pdev->devfn & 0xff) << 8;
> +            bdf = (b << 16) | (df << 8);
> 
>             if ( unlikely(copy_to_guest_offset(buf, i, &bdf, 1)) )
>             {
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -145,7 +145,7 @@ unsigned int iommu_read_apic_from_ire(un
> int __init iommu_setup_hpet_msi(struct msi_desc *msi)
> {
>     const struct iommu_ops *ops = iommu_get_ops();
> -    return ops->setup_hpet_msi ? ops->setup_hpet_msi(msi) : -ENODEV;
> +    return ops->setup_hpet_msi ? iommu_call(ops, setup_hpet_msi, msi) : 
> -ENODEV;
> }
> 
> void __hwdom_init arch_iommu_check_autotranslated_hwdom(struct domain *d)
> @@ -406,7 +406,7 @@ int iommu_free_pgtables(struct domain *d
>      * Pages will be moved to the free list below. So we want to
>      * clear the root page-table to avoid any potential use after-free.
>      */
> -    hd->platform_ops->clear_root_pgtable(d);
> +    iommu_vcall(hd->platform_ops, clear_root_pgtable, d);
> 
>     while ( (pg = page_list_remove_head(&hd->arch.pgtables.list)) )
>     {
> 
> 




 


Rackspace

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