| Message ID | 1505999845-12577-5-git-send-email-mjaggi@caviumnetworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote: > From: Manish Jaggi <mjaggi@cavium.com> > > estimate_acpi_efi_size needs to be updated to provide correct size of > hardware domains MADT, which now adds ITS information as well. > > Introducing gic_get_hwdom_madt_size. I think the commit title is misleading, the main purpose of this patch is updating the formula to compute the MADT size for GICv3. Not introducing the callbacks. But likely, you want two patches here: - Patch #1 adding the callbacks - Patch #2 updating the formula for GICv3 For this time, I would be ok to have only one patch providing the commit message is updated. Cheers,
Hi On 10/3/2017 8:01 PM, Julien Grall wrote: > Hi, > > On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote: >> From: Manish Jaggi <mjaggi@cavium.com> >> >> estimate_acpi_efi_size needs to be updated to provide correct size of >> hardware domains MADT, which now adds ITS information as well. >> >> Introducing gic_get_hwdom_madt_size. > > I think the commit title is misleading, the main purpose of this patch > is updating the formula to compute the MADT size for GICv3. Not > introducing the callbacks. > > But likely, you want two patches here: > - Patch #1 adding the callbacks > - Patch #2 updating the formula for GICv3 > > For this time, I would be ok to have only one patch providing the > commit message is updated. > ok, will update.. > Cheers, >
Hi, On 21/09/17 14:17, mjaggi@caviumnetworks.com wrote: > From: Manish Jaggi <mjaggi@cavium.com> > > estimate_acpi_efi_size needs to be updated to provide correct size of > hardware domains MADT, which now adds ITS information as well. > > Introducing gic_get_hwdom_madt_size. > > Signed-off-by: Manish Jaggi <mjaggi@cavium.com> > --- > xen/arch/arm/domain_build.c | 7 +------ > xen/arch/arm/gic-v2.c | 9 +++++++++ > xen/arch/arm/gic-v3.c | 19 +++++++++++++++++++ > xen/arch/arm/gic.c | 12 ++++++++++++ > xen/include/asm-arm/gic.h | 3 +++ > 5 files changed, 44 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index d6f9585..f17fcf1 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1808,12 +1808,7 @@ static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo) > acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8); > acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8); > > - madt_size = sizeof(struct acpi_table_madt) > - + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus > - + sizeof(struct acpi_madt_generic_distributor); > - if ( d->arch.vgic.version == GIC_V3 ) > - madt_size += sizeof(struct acpi_madt_generic_redistributor) > - * d->arch.vgic.nr_regions; > + madt_size = gic_get_hwdom_madt_size(d); > acpi_size += ROUNDUP(madt_size, 8); > > addr = acpi_os_get_root_pointer(); > diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c > index cbe71a9..2868766 100644 > --- a/xen/arch/arm/gic-v2.c > +++ b/xen/arch/arm/gic-v2.c > @@ -1012,6 +1012,14 @@ static int gicv2_iomem_deny_access(const struct domain *d) > return iomem_deny_access(d, mfn, mfn + nr); > } > > +static unsigned long gicv2_get_hwdom_madt_size(const struct domain *d) > +{ > + return sizeof(struct acpi_table_madt) > + + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus > + + sizeof(struct acpi_madt_generic_distributor); > + > +} > + > #ifdef CONFIG_ACPI > static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset) > { > @@ -1248,6 +1256,7 @@ const static struct gic_hw_operations gicv2_ops = { > .read_apr = gicv2_read_apr, > .make_hwdom_dt_node = gicv2_make_hwdom_dt_node, > .make_hwdom_madt = gicv2_make_hwdom_madt, > + .get_hwdom_madt_size = gicv2_get_hwdom_madt_size, > .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings, > .iomem_deny_access = gicv2_iomem_deny_access, > .do_LPI = gicv2_do_LPI, > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index b3d605d..6e8d580 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1406,6 +1406,19 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) > return table_len; > } > > +static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d) > +{ > + unsigned long size; > + > + size = sizeof(struct acpi_madt_generic_redistributor) > + * d->arch.vgic.nr_regions; > + > + size += vgic_v3_its_count(d) > + * sizeof(struct acpi_madt_generic_translator); > + > + return size; > +} > + > static int __init > gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, > const unsigned long end) > @@ -1597,6 +1610,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) > { > return 0; > } > + > +static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d) > +{ > + return 0; > +} > #endif > > /* Set up the GIC */ > @@ -1698,6 +1716,7 @@ static const struct gic_hw_operations gicv3_ops = { > .secondary_init = gicv3_secondary_cpu_init, > .make_hwdom_dt_node = gicv3_make_hwdom_dt_node, > .make_hwdom_madt = gicv3_make_hwdom_madt, > + .get_hwdom_madt_size = gicv3_get_hwdom_madt_size, > .iomem_deny_access = gicv3_iomem_deny_access, > .do_LPI = gicv3_do_LPI, > }; > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 6c803bf..f3c1f0b 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -851,6 +851,18 @@ int gic_make_hwdom_madt(const struct domain *d, u32 offset) > return gic_hw_ops->make_hwdom_madt(d, offset); > } > > +unsigned long gic_get_hwdom_madt_size(const struct domain *d) > +{ > + unsigned long madt_size; > + > + madt_size = sizeof(struct acpi_table_madt) > + + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus > + + sizeof(struct acpi_madt_generic_distributor) > + + gic_hw_ops->get_hwdom_madt_size(d); But this is now doubled for a GICv2? As you already do that calculation in the GICv2 callback? So I suggest you drop that *there* and rename the function member to get_hwdom_extra_madt_size() (or so). So in the GICv2 version you can now return 0. Keep the GICv3 version as it is. That solution should be both readable and avoiding code duplication. Cheers, Andre. > + > + return madt_size; > +} > + > int gic_iomem_deny_access(const struct domain *d) > { > return gic_hw_ops->iomem_deny_access(d); > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index 6203dc5..3acdd6d 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -365,6 +365,8 @@ struct gic_hw_operations { > int (*make_hwdom_madt)(const struct domain *d, u32 offset); > /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */ > int (*map_hwdom_extra_mappings)(struct domain *d); > + /* Query the size of hardware domain madt table */ > + unsigned long (*get_hwdom_madt_size)(const struct domain *d); > /* Deny access to GIC regions */ > int (*iomem_deny_access)(const struct domain *d); > /* Handle LPIs, which require special handling */ > @@ -376,6 +378,7 @@ int gic_make_hwdom_dt_node(const struct domain *d, > const struct dt_device_node *gic, > void *fdt); > int gic_make_hwdom_madt(const struct domain *d, u32 offset); > +unsigned long gic_get_hwdom_madt_size(const struct domain *d); > int gic_map_hwdom_extra_mappings(struct domain *d); > int gic_iomem_deny_access(const struct domain *d); > >
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index d6f9585..f17fcf1 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1808,12 +1808,7 @@ static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo) acpi_size = ROUNDUP(sizeof(struct acpi_table_fadt), 8); acpi_size += ROUNDUP(sizeof(struct acpi_table_stao), 8); - madt_size = sizeof(struct acpi_table_madt) - + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus - + sizeof(struct acpi_madt_generic_distributor); - if ( d->arch.vgic.version == GIC_V3 ) - madt_size += sizeof(struct acpi_madt_generic_redistributor) - * d->arch.vgic.nr_regions; + madt_size = gic_get_hwdom_madt_size(d); acpi_size += ROUNDUP(madt_size, 8); addr = acpi_os_get_root_pointer(); diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c index cbe71a9..2868766 100644 --- a/xen/arch/arm/gic-v2.c +++ b/xen/arch/arm/gic-v2.c @@ -1012,6 +1012,14 @@ static int gicv2_iomem_deny_access(const struct domain *d) return iomem_deny_access(d, mfn, mfn + nr); } +static unsigned long gicv2_get_hwdom_madt_size(const struct domain *d) +{ + return sizeof(struct acpi_table_madt) + + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus + + sizeof(struct acpi_madt_generic_distributor); + +} + #ifdef CONFIG_ACPI static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset) { @@ -1248,6 +1256,7 @@ const static struct gic_hw_operations gicv2_ops = { .read_apr = gicv2_read_apr, .make_hwdom_dt_node = gicv2_make_hwdom_dt_node, .make_hwdom_madt = gicv2_make_hwdom_madt, + .get_hwdom_madt_size = gicv2_get_hwdom_madt_size, .map_hwdom_extra_mappings = gicv2_map_hwdown_extra_mappings, .iomem_deny_access = gicv2_iomem_deny_access, .do_LPI = gicv2_do_LPI, diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index b3d605d..6e8d580 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -1406,6 +1406,19 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) return table_len; } +static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d) +{ + unsigned long size; + + size = sizeof(struct acpi_madt_generic_redistributor) + * d->arch.vgic.nr_regions; + + size += vgic_v3_its_count(d) + * sizeof(struct acpi_madt_generic_translator); + + return size; +} + static int __init gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header, const unsigned long end) @@ -1597,6 +1610,11 @@ static int gicv3_make_hwdom_madt(const struct domain *d, u32 offset) { return 0; } + +static unsigned long gicv3_get_hwdom_madt_size(const struct domain *d) +{ + return 0; +} #endif /* Set up the GIC */ @@ -1698,6 +1716,7 @@ static const struct gic_hw_operations gicv3_ops = { .secondary_init = gicv3_secondary_cpu_init, .make_hwdom_dt_node = gicv3_make_hwdom_dt_node, .make_hwdom_madt = gicv3_make_hwdom_madt, + .get_hwdom_madt_size = gicv3_get_hwdom_madt_size, .iomem_deny_access = gicv3_iomem_deny_access, .do_LPI = gicv3_do_LPI, }; diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 6c803bf..f3c1f0b 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -851,6 +851,18 @@ int gic_make_hwdom_madt(const struct domain *d, u32 offset) return gic_hw_ops->make_hwdom_madt(d, offset); } +unsigned long gic_get_hwdom_madt_size(const struct domain *d) +{ + unsigned long madt_size; + + madt_size = sizeof(struct acpi_table_madt) + + sizeof(struct acpi_madt_generic_interrupt) * d->max_vcpus + + sizeof(struct acpi_madt_generic_distributor) + + gic_hw_ops->get_hwdom_madt_size(d); + + return madt_size; +} + int gic_iomem_deny_access(const struct domain *d) { return gic_hw_ops->iomem_deny_access(d); diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index 6203dc5..3acdd6d 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -365,6 +365,8 @@ struct gic_hw_operations { int (*make_hwdom_madt)(const struct domain *d, u32 offset); /* Map extra GIC MMIO, irqs and other hw stuffs to the hardware domain. */ int (*map_hwdom_extra_mappings)(struct domain *d); + /* Query the size of hardware domain madt table */ + unsigned long (*get_hwdom_madt_size)(const struct domain *d); /* Deny access to GIC regions */ int (*iomem_deny_access)(const struct domain *d); /* Handle LPIs, which require special handling */ @@ -376,6 +378,7 @@ int gic_make_hwdom_dt_node(const struct domain *d, const struct dt_device_node *gic, void *fdt); int gic_make_hwdom_madt(const struct domain *d, u32 offset); +unsigned long gic_get_hwdom_madt_size(const struct domain *d); int gic_map_hwdom_extra_mappings(struct domain *d); int gic_iomem_deny_access(const struct domain *d);