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

Re: [Xen-devel] [RFC 02/11] acpi: arm: API to query estimated size of hardware domain's IORT


On 19/01/18 06:10, Manish Jaggi wrote:

On 01/17/2018 12:22 AM, Julien Grall wrote:
  IORT for hardware domain is generated using the requesterId and deviceId map.

  Signed-off-by: Manish Jaggi <manish.jaggi@xxxxxxxxxx>
  xen/arch/arm/domain_build.c     |  12 ++++-
  xen/drivers/acpi/arm/Makefile   |   1 +
  xen/drivers/acpi/arm/gen-iort.c | 101 ++++++++++++++++++++++++++++++++++++++++
  xen/include/acpi/gen-iort.h     |   6 +++
  4 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index c74f4dd69d..f5d5e3d271 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -14,6 +14,7 @@
  #include <xen/acpi.h>
  #include <xen/warning.h>
  #include <acpi/actables.h>
+#include <acpi/gen-iort.h>
  #include <asm/device.h>
  #include <asm/setup.h>
  #include <asm/platform.h>
@@ -1799,7 +1800,7 @@ static int acpi_create_fadt(struct domain *d, struct membank tbl_add[])     static int estimate_acpi_efi_size(struct domain *d, struct kernel_info *kinfo)
-    size_t efi_size, acpi_size, madt_size;
+    size_t efi_size, acpi_size, madt_size, iort_size;

Rather than introduce a variable for 10 instructions, you can rename madt_size so it can be re-used. I would be ok for this to be in the same patch (providing a proper commit message).
Why would you want to replace iort_size with madt_size ?
What is the harm if adding a variable makes the code more verbose.
I am not able to appreciate your point here.

I didn't ask to replace iort_size with madt_size. But rename madt_size to table_size or some other name that could be reused for both.

This is very similar to when you store the error return of a function. You are not going to name ret_foo, ret_bar, ret_fish... You are just going to use one variable and re-use it.

Anyway, I am not going to fight with that and just send a patch to clean that up once it has been merged.


  diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
index 046fad5e3d..13f1a9159f 100644
--- a/xen/drivers/acpi/arm/Makefile
+++ b/xen/drivers/acpi/arm/Makefile
@@ -1 +1,2 @@
  obj-y = ridmap.o
+obj-y += gen-iort.o
diff --git a/xen/drivers/acpi/arm/gen-iort.c b/xen/drivers/acpi/arm/gen-iort.c
new file mode 100644
index 0000000000..3fc32959c6
--- /dev/null
+++ b/xen/drivers/acpi/arm/gen-iort.c
@@ -0,0 +1,101 @@
+ * xen/drivers/acpi/arm/gen-iort.c
+ *
+ * Code to generate IORT for hardware domain using the requesterId
+ * and deviceId map.
+ *
+ * Manish Jaggi <manish.jaggi@xxxxxxxxxx>
+ * Copyright (c) 2018 Linaro.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.

The license is wrong (see patch #1).
Please see my comment in patch #1.
This license is used from an existing file in xen.
So there are a lot of wrong licenses in xen code.

Well yes. But does it mean you have to add more wrong code? ;)


Julien Grall

Xen-devel mailing list



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