| Message ID | 20211116063155.901183-9-penny.zheng@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | direct-map memory map | expand |
Hi Penny, On 16/11/2021 06:31, Penny Zheng wrote: > From: Stefano Stabellini <stefano.stabellini@xilinx.com> > > Today we use native addresses to map the GICv3 for Dom0 and fixed > addresses for DomUs. > > This patch changes the behavior so that native addresses are used for > all domain which is using the host memory layout > > Considering that DOM0 may not always be directly mapped in the future, > this patch introduces a new helper "domain_use_host_layout()" that > wraps both two check "is_domain_direct_mapped(d) || is_hardware_domain(d)" > for more flexible usage. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> Same remark as another patch about the order of the signed-off-by. > --- > v2 changes: > - remove redistributor accessor > - introduce new helper "is_domain_use_host_layout()" > - comment fix > --- > v3 changes: > - the comment on top of 'buf' to explain how 38 was found > - fix res getting overwritten > - drop 'cells += (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS)' > - free 'reg' right way > - fix comment > - rename 'is_domain_use_host_layout()' to 'domain_use_host_layout()' > --- > xen/arch/arm/domain_build.c | 37 +++++++++++++++++++++++++++--------- > xen/arch/arm/vgic-v3.c | 29 ++++++++++++++++------------ > xen/include/asm-arm/domain.h | 7 +++++++ > 3 files changed, 52 insertions(+), 21 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 24f3edf069..61fd374c5d 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -2284,10 +2284,16 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo) > { > void *fdt = kinfo->fdt; > int res = 0; > - __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2]; > - __be32 *cells; > + __be32 *reg; > + const struct domain *d = kinfo->d; > + /* Placeholder for interrupt-controller@ + a 64-bit number + \0 */ > + char buf[38]; > + unsigned int i, len = 0; > > - res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICV3_GICD_BASE)); > + snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64, > + vgic_dist_base(&d->arch.vgic)); > + > + res = fdt_begin_node(fdt, buf); > if ( res ) > return res; > > @@ -2307,13 +2313,26 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo) > if ( res ) > return res; > > - cells = ®[0]; > - dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, > - GUEST_GICV3_GICD_BASE, GUEST_GICV3_GICD_SIZE); > - dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, > - GUEST_GICV3_GICR0_BASE, GUEST_GICV3_GICR0_SIZE); > + /* reg specifies all re-distributors and Distributor. */ > + len = (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * > + (d->arch.vgic.nr_regions + 1) * sizeof(__be32); > + reg = xmalloc_bytes(len); > + if ( reg == NULL ) > + return -ENOMEM; > > - res = fdt_property(fdt, "reg", reg, sizeof(reg)); > + dt_child_set_range(®, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, > + vgic_dist_base(&d->arch.vgic), GUEST_GICV3_GICD_SIZE); > + > + for ( i = 0; i < d->arch.vgic.nr_regions; i++) > + { > + dt_child_set_range(®, > + GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, > + d->arch.vgic.rdist_regions[i].base, > + d->arch.vgic.rdist_regions[i].size); > + } > + > + res = fdt_property(fdt, "reg", reg, len); > + xfree(reg); > if (res) > return res; > > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c > index 65bb7991a6..181b66513d 100644 > --- a/xen/arch/arm/vgic-v3.c > +++ b/xen/arch/arm/vgic-v3.c > @@ -1640,14 +1640,15 @@ static inline unsigned int vgic_v3_max_rdist_count(struct domain *d) > * Normally there is only one GICv3 redistributor region. > * The GICv3 DT binding provisions for multiple regions, since there are > * platforms out there which need those (multi-socket systems). > - * For Dom0 we have to live with the MMIO layout the hardware provides, > - * so we have to copy the multiple regions - as the first region may not > - * provide enough space to hold all redistributors we need. > + * For domain using the host memory layout, we have to live with the MMIO > + * layout the hardware provides, so we have to copy the multiple regions > + * - as the first region may not provide enough space to hold all > + * redistributors we need. > * However DomU get a constructed memory map, so we can go with > * the architected single redistributor region. > */ > - return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions : > - GUEST_GICV3_RDIST_REGIONS; > + return domain_use_host_layout(d) ? vgic_v3_hw.nr_rdist_regions : > + GUEST_GICV3_RDIST_REGIONS; > } > > static int vgic_v3_domain_init(struct domain *d) > @@ -1669,10 +1670,14 @@ static int vgic_v3_domain_init(struct domain *d) > radix_tree_init(&d->arch.vgic.pend_lpi_tree); > > /* > - * Domain 0 gets the hardware address. > - * Guests get the virtual platform layout. > + * Since we map the whole GICv3 register memory map(64KB) for > + * all domain, DOM0 and direct-map domain could be treated the > + * same way here. I find this confusing because it is not clear what you refer to with the "register memory map" here (I think you mean the Distributor). That said, I would drop this paragraph as what matters is we are using the same layout as the host. > + * For domain using the host memory layout, it gets the hardware > + * address. > + * Other domains get the virtual platform layout. > */ > - if ( is_hardware_domain(d) ) > + if ( domain_use_host_layout(d) ) > { > unsigned int first_cpu = 0; > > @@ -1695,10 +1700,10 @@ static int vgic_v3_domain_init(struct domain *d) > } > > /* > - * The hardware domain may not use all the re-distributors > - * regions (e.g when the number of vCPUs does not match the > - * number of pCPUs). Update the number of regions to avoid > - * exposing unused region as they will not get emulated. > + * For domain using the host memory layout, it may not use all > + * the re-distributors regions (e.g when the number of vCPUs does > + * not match the number of pCPUs). Update the number of regions to > + * avoid exposing unused region as they will not get emulated. > */ > d->arch.vgic.nr_regions = i + 1; > > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 4f2c3f09d4..0eff93197e 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -32,6 +32,13 @@ enum domain_type { > #define is_domain_direct_mapped(d) \ > (d->options & XEN_DOMCTL_CDF_INTERNAL_directmap) > > +/* > + * For domain using the host memory layout, we have to live with the MMIO > + * layout the hardware provides. > + */ How about: /* * Is the domain using the host memory layout? * * Direct-mapped domain will always have the RAM mapped with GFN == MFN. * To avoid any trouble finding space, it is easier to force using the * host memory layout. * * The hardware domain will use the host layout regardless of * direct-mapped because some OS may rely on a specific address ranges * for the devices. */ > +#define domain_use_host_layout(d) (is_domain_direct_mapped(d) || \ > + is_hardware_domain(d)) > + > struct vtimer { > struct vcpu *v; > int irq; > Cheers,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 24f3edf069..61fd374c5d 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -2284,10 +2284,16 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo) { void *fdt = kinfo->fdt; int res = 0; - __be32 reg[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * 2]; - __be32 *cells; + __be32 *reg; + const struct domain *d = kinfo->d; + /* Placeholder for interrupt-controller@ + a 64-bit number + \0 */ + char buf[38]; + unsigned int i, len = 0; - res = fdt_begin_node(fdt, "interrupt-controller@"__stringify(GUEST_GICV3_GICD_BASE)); + snprintf(buf, sizeof(buf), "interrupt-controller@%"PRIx64, + vgic_dist_base(&d->arch.vgic)); + + res = fdt_begin_node(fdt, buf); if ( res ) return res; @@ -2307,13 +2313,26 @@ static int __init make_gicv3_domU_node(struct kernel_info *kinfo) if ( res ) return res; - cells = ®[0]; - dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, - GUEST_GICV3_GICD_BASE, GUEST_GICV3_GICD_SIZE); - dt_child_set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, - GUEST_GICV3_GICR0_BASE, GUEST_GICV3_GICR0_SIZE); + /* reg specifies all re-distributors and Distributor. */ + len = (GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) * + (d->arch.vgic.nr_regions + 1) * sizeof(__be32); + reg = xmalloc_bytes(len); + if ( reg == NULL ) + return -ENOMEM; - res = fdt_property(fdt, "reg", reg, sizeof(reg)); + dt_child_set_range(®, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, + vgic_dist_base(&d->arch.vgic), GUEST_GICV3_GICD_SIZE); + + for ( i = 0; i < d->arch.vgic.nr_regions; i++) + { + dt_child_set_range(®, + GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS, + d->arch.vgic.rdist_regions[i].base, + d->arch.vgic.rdist_regions[i].size); + } + + res = fdt_property(fdt, "reg", reg, len); + xfree(reg); if (res) return res; diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index 65bb7991a6..181b66513d 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -1640,14 +1640,15 @@ static inline unsigned int vgic_v3_max_rdist_count(struct domain *d) * Normally there is only one GICv3 redistributor region. * The GICv3 DT binding provisions for multiple regions, since there are * platforms out there which need those (multi-socket systems). - * For Dom0 we have to live with the MMIO layout the hardware provides, - * so we have to copy the multiple regions - as the first region may not - * provide enough space to hold all redistributors we need. + * For domain using the host memory layout, we have to live with the MMIO + * layout the hardware provides, so we have to copy the multiple regions + * - as the first region may not provide enough space to hold all + * redistributors we need. * However DomU get a constructed memory map, so we can go with * the architected single redistributor region. */ - return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions : - GUEST_GICV3_RDIST_REGIONS; + return domain_use_host_layout(d) ? vgic_v3_hw.nr_rdist_regions : + GUEST_GICV3_RDIST_REGIONS; } static int vgic_v3_domain_init(struct domain *d) @@ -1669,10 +1670,14 @@ static int vgic_v3_domain_init(struct domain *d) radix_tree_init(&d->arch.vgic.pend_lpi_tree); /* - * Domain 0 gets the hardware address. - * Guests get the virtual platform layout. + * Since we map the whole GICv3 register memory map(64KB) for + * all domain, DOM0 and direct-map domain could be treated the + * same way here. + * For domain using the host memory layout, it gets the hardware + * address. + * Other domains get the virtual platform layout. */ - if ( is_hardware_domain(d) ) + if ( domain_use_host_layout(d) ) { unsigned int first_cpu = 0; @@ -1695,10 +1700,10 @@ static int vgic_v3_domain_init(struct domain *d) } /* - * The hardware domain may not use all the re-distributors - * regions (e.g when the number of vCPUs does not match the - * number of pCPUs). Update the number of regions to avoid - * exposing unused region as they will not get emulated. + * For domain using the host memory layout, it may not use all + * the re-distributors regions (e.g when the number of vCPUs does + * not match the number of pCPUs). Update the number of regions to + * avoid exposing unused region as they will not get emulated. */ d->arch.vgic.nr_regions = i + 1; diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 4f2c3f09d4..0eff93197e 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -32,6 +32,13 @@ enum domain_type { #define is_domain_direct_mapped(d) \ (d->options & XEN_DOMCTL_CDF_INTERNAL_directmap) +/* + * For domain using the host memory layout, we have to live with the MMIO + * layout the hardware provides. + */ +#define domain_use_host_layout(d) (is_domain_direct_mapped(d) || \ + is_hardware_domain(d)) + struct vtimer { struct vcpu *v; int irq;