Re: [PATCH v4 3/8] x86/domain: allocate d->{iomem,irq}_caps before arch-specific initialization

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

Hi Denis,

On 31.07.25 22:21, dmkhn@xxxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
>
> Move IRQ/IOMEM rangesets allocation before arch_domain_create().
>
> That guarantees that arch-specific code could access those rangesets to
> register traps for emulation.
>
> It is necessary for those emulators registering trap handlers and ensuring
> that emulated IRQs are not shared with the physical IRQs.
>
> Move dom0_setup_permissions() call right after I/O rangesets are allocated.
>
> Move pvh_setup_mmcfg() inside dom0_setup_permissions() close to the place
> where MMCFG ranges are initialized.
>
> Not a functional change.
>
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> ---
> Chanhes since v3:
> - new patch
> ---
>   xen/arch/x86/dom0_build.c     | 26 +++++++++++++++++++++++
>   xen/arch/x86/hvm/dom0_build.c | 39 -----------------------------------
>   xen/arch/x86/hvm/hvm.c        | 16 ++++++++++++++
>   xen/common/domain.c           | 12 +++++------
>   4 files changed, 48 insertions(+), 45 deletions(-)
>
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 0b467fd4a4fc..e965f506a3c8 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -471,6 +471,24 @@ static void __init process_dom0_ioports_disable(struct 
> domain *dom0)
>       }
>   }
>   
> +static void __hwdom_init setup_mmcfg(struct domain *d)
> +{
> +    unsigned int i;
> +    int rc;
> +
> +    for ( i = 0; i < pci_mmcfg_config_num; i++ )
> +    {
> +        rc = register_vpci_mmcfg_handler(d, pci_mmcfg_config[i].address,
> +                                         pci_mmcfg_config[i].start_bus_number,
> +                                         pci_mmcfg_config[i].end_bus_number,
> +                                         pci_mmcfg_config[i].pci_segment);
> +        if ( rc )
> +            printk("Unable to setup MMCFG handler at %#lx for segment %u\n",
> +                   pci_mmcfg_config[i].address,
> +                   pci_mmcfg_config[i].pci_segment);
> +    }
> +}
> +
>   int __init dom0_setup_permissions(struct domain *d)
It could be i'm missing smth, but ^ function is __init while ...

>   {
>       unsigned long mfn;
> @@ -480,6 +498,14 @@ int __init dom0_setup_permissions(struct domain *d)
>       if ( pv_shim )
>           return 0;
>   
> +    /*
> +     * MMCFG initialization must be performed before setting domain
> +     * permissions, as the MCFG areas must not be part of the domain IOMEM
> +     * accessible regions.
> +     */
> +    if ( is_hvm_domain(d) )
> +        setup_mmcfg(d);
> +
>       /* The hardware domain is initially permitted full I/O capabilities. */
>       rc = ioports_permit_access(d, 0, 0xFFFF);
>       rc |= iomem_permit_access(d, 0UL,
> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index 5551f9044836..6f47c9eeeaa6 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -1310,24 +1310,6 @@ static int __init pvh_setup_acpi(struct domain *d, 
> paddr_t start_info)
>       return 0;
>   }
>   
> -static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
> -{
> -    unsigned int i;
> -    int rc;
> -
> -    for ( i = 0; i < pci_mmcfg_config_num; i++ )
> -    {
> -        rc = register_vpci_mmcfg_handler(d, pci_mmcfg_config[i].address,
> -                                         pci_mmcfg_config[i].start_bus_number,
> -                                         pci_mmcfg_config[i].end_bus_number,
> -                                         pci_mmcfg_config[i].pci_segment);
> -        if ( rc )
> -            printk("Unable to setup MMCFG handler at %#lx for segment %u\n",
> -                   pci_mmcfg_config[i].address,
> -                   pci_mmcfg_config[i].pci_segment);
> -    }
> -}
> -
>   int __init dom0_construct_pvh(const struct boot_domain *bd)
>   {
>       paddr_t entry, start_info;
> @@ -1339,27 +1321,6 @@ int __init dom0_construct_pvh(const struct boot_domain 
> *bd)
>       if ( bd->kernel == NULL )
>           panic("Missing kernel boot module for %pd construction\n", d);
>   
> -    if ( is_hardware_domain(d) )
> -    {
> -        /*
> -         * MMCFG initialization must be performed before setting domain
> -         * permissions, as the MCFG areas must not be part of the domain IOMEM
> -         * accessible regions.
> -         */
> -        pvh_setup_mmcfg(d);
> -
> -        /*
> -         * Setup permissions early so that calls to add MMIO regions to the
> -         * p2m as part of vPCI setup don't fail due to permission checks.
> -         */
> -        rc = dom0_setup_permissions(d);
> -        if ( rc )
> -        {
> -            printk("%pd unable to setup permissions: %d\n", d, rc);
> -            return rc;
> -        }
> -    }
> -
>       /*
>        * Craft dom0 physical memory map and set the paging allocation. This must
>        * be done before the iommu initializion, since iommu initialization code
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index cb8ecd050d41..b7edb1d6555d 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -35,6 +35,7 @@
>   #include <asm/hap.h>
>   #include <asm/current.h>
>   #include <asm/debugreg.h>
> +#include <asm/dom0_build.h>
>   #include <asm/e820.h>
>   #include <asm/regs.h>
>   #include <asm/cpufeature.h>
> @@ -651,6 +652,17 @@ int hvm_domain_initialise(struct domain *d,
>               goto fail1;
>           }
>           memset(d->arch.hvm.io_bitmap, ~0, HVM_IOBITMAP_SIZE);
> +
> +        /*
> +         * Setup permissions early so that calls to add MMIO regions to the
> +         * p2m as part of vPCI setup don't fail due to permission checks.
> +         */
> +        rc = dom0_setup_permissions(d);
... here hvm_domain_initialise() is not __init?

[...]
 

--
Best regards,
-grygorii