Re: [Xen-devel] [PATCH v5 4/5] ARM: Update Formula to compute MADT size using new callbacks in gic_hw_operations

[Julien Grall]

Hi,

On 10/10/17 11:33, Manish Jaggi wrote:


On 10/10/2017 3:44 PM, Julien Grall wrote:
Hi Manish,

On 10/10/17 07:16, mja...@caviumnetworks.com wrote:
From: Manish Jaggi <mja...@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.

This patch updates the formula to compute extra MADT size, as per 
GICv2/3
by calling gic_get_hwdom_extra_madt_size

Missing full stop.
oh i missed it.


Signed-off-by: Manish Jaggi <mja...@cavium.com>
---
  xen/arch/arm/domain_build.c |  7 +------
  xen/arch/arm/gic-v2.c       |  6 ++++++
  xen/arch/arm/gic-v3.c       | 19 +++++++++++++++++++
  xen/arch/arm/gic.c          | 12 ++++++++++++
  xen/include/asm-arm/gic.h   |  3 +++
  5 files changed, 41 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..0123ea4 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -1012,6 +1012,11 @@ static int gicv2_iomem_deny_access(const 
struct domain *d)
      return iomem_deny_access(d, mfn, mfn + nr);
  }
  +static unsigned long gicv2_get_hwdom_extra_madt_size(const struct 
domain *d)
+{
+    return 0;
+}
+
  #ifdef CONFIG_ACPI
  static int gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
  {
@@ -1248,6 +1253,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_extra_madt_size = gicv2_get_hwdom_extra_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..447998d 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_extra_madt_size(const struct 
domain *d)
+{
+    unsigned long size;
+
+    size  = sizeof(struct acpi_madt_generic_redistributor)
+                    * d->arch.vgic.nr_regions;

Here you align the * with struct. But below, you align with sizeof. 
Please stay consistent and always align with sizeof.

+
+    size  += vgic_v3_its_count(d)
+                    * sizeof(struct acpi_madt_generic_translator);

Same here.

Could you please help with the specific section on coding style 
guidelines on xen for indentation when line over 80 chars which I am not 
following for this case.

The best guideline is the existing code around in the file, in doubt ask.

The code you copied was aligned with sizeof (see in domain_build.c) and 
now it does not have the same alignment.

Furthermore, you used double tab and not one tab (Xen is using 4 spaces).

Cheers

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel