Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-11-15 Thread Julien Grall 

Hi Sameer,

On 11/15/2017 01:27 AM, Goel, Sameer wrote:



On 11/8/2017 7:41 AM, Manish Jaggi wrote:

Hi Sameer

On 9/21/2017 6:07 AM, Sameer Goel wrote:

Add support for parsing IORT table to initialize SMMU devices.
* The code for creating an SMMU device has been modified, so that the SMMU
device can be initialized.
* The NAMED NODE code has been commented out as this will need DOM0 kernel
support.
* ITS code has been included but it has not been tested.

Signed-off-by: Sameer Goel 

Followup of the discussions we had on iort parsing and querying streamID and 
deviceId based on RID.
I have extended your patchset with a patch that provides an alternative
way of parsing iort into maps : {rid-streamid}, {rid-deviceID)
which can directly be looked up for searching streamId for a rid. This
will remove the need to traverse iort table again.

The test patch just describes the proposed flow and how the parsing and
query code might fit in. I have not tested it.
The code only compiles.

https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5
(This repo has all 7 of your patches + test code patch merged.

Note: The commit text of the patch describes the basic flow /assumptions / 
usage of functions.
Please see the code along with the v2 design draft.
[RFC] [Draft Design v2] ACPI/IORT Support in Xen.
https://lists.xen.org/archives/html/xen-devel/2017-11/msg00512.html

I seek your advice on this. Please provide your feedback.

I responded back on the other thread. I think we are fixing something that is 
not broken. I will try to post a couple of new RFCs and let's discuss this with 
incremental changes on the mailing list.


That other thread was a separate mailing list. For the benefits of the 
rest of the community it would be nice if you can post the summary here.


However, nobody said the code was broken. IORT will be used in various 
place in Xen and Manish is looking whether we can parse only once and 
for all the IORT. I think it is latest design is promising for that.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-11-14 Thread Goel, Sameer 


On 11/8/2017 7:41 AM, Manish Jaggi wrote:
> Hi Sameer
> 
> On 9/21/2017 6:07 AM, Sameer Goel wrote:
>> Add support for parsing IORT table to initialize SMMU devices.
>> * The code for creating an SMMU device has been modified, so that the SMMU
>> device can be initialized.
>> * The NAMED NODE code has been commented out as this will need DOM0 kernel
>> support.
>> * ITS code has been included but it has not been tested.
>>
>> Signed-off-by: Sameer Goel 
> Followup of the discussions we had on iort parsing and querying streamID and 
> deviceId based on RID.
> I have extended your patchset with a patch that provides an alternative
> way of parsing iort into maps : {rid-streamid}, {rid-deviceID)
> which can directly be looked up for searching streamId for a rid. This
> will remove the need to traverse iort table again.
> 
> The test patch just describes the proposed flow and how the parsing and
> query code might fit in. I have not tested it.
> The code only compiles.
> 
> https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5
> (This repo has all 7 of your patches + test code patch merged.
> 
> Note: The commit text of the patch describes the basic flow /assumptions / 
> usage of functions.
> Please see the code along with the v2 design draft.
> [RFC] [Draft Design v2] ACPI/IORT Support in Xen.
> https://lists.xen.org/archives/html/xen-devel/2017-11/msg00512.html
> 
> I seek your advice on this. Please provide your feedback.
I responded back on the other thread. I think we are fixing something that is 
not broken. I will try to post a couple of new RFCs and let's discuss this with 
incremental changes on the mailing list.

Thanks,
Sameer
> 
> Thanks
> Manish
> 
> 
>> ---
>>   xen/arch/arm/setup.c   |   3 +
>>   xen/drivers/acpi/Makefile  |   1 +
>>   xen/drivers/acpi/arm/Makefile  |   1 +
>>   xen/drivers/acpi/arm/iort.c    | 173 
>> +
>>   xen/drivers/passthrough/arm/smmu.c |   1 +
>>   xen/include/acpi/acpi_iort.h   |  17 ++--
>>   xen/include/asm-arm/device.h   |   2 +
>>   xen/include/xen/acpi.h |  21 +
>>   xen/include/xen/pci.h  |   8 ++
>>   9 files changed, 146 insertions(+), 81 deletions(-)
>>   create mode 100644 xen/drivers/acpi/arm/Makefile
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 92f173b..4ba09b2 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -49,6 +49,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>     struct bootinfo __initdata bootinfo;
>>   @@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>>     tasklet_subsys_init();
>>   +    /* Parse the ACPI iort data */
>> +    acpi_iort_init();
>>     xsm_dt_init();
>>   diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
>> index 444b11d..e7ffd82 100644
>> --- a/xen/drivers/acpi/Makefile
>> +++ b/xen/drivers/acpi/Makefile
>> @@ -1,5 +1,6 @@
>>   subdir-y += tables
>>   subdir-y += utilities
>> +subdir-$(CONFIG_ARM) += arm
>>   subdir-$(CONFIG_X86) += apei
>>     obj-bin-y += tables.init.o
>> diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
>> new file mode 100644
>> index 000..7c039bb
>> --- /dev/null
>> +++ b/xen/drivers/acpi/arm/Makefile
>> @@ -0,0 +1 @@
>> +obj-y += iort.o
>> diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
>> index 2e368a6..7f54062 100644
>> --- a/xen/drivers/acpi/arm/iort.c
>> +++ b/xen/drivers/acpi/arm/iort.c
>> @@ -14,17 +14,47 @@
>>    * This file implements early detection/parsing of I/O mapping
>>    * reported to OS through firmware via I/O Remapping Table (IORT)
>>    * IORT document number: ARM DEN 0049A
>> + *
>> + * Based on Linux drivers/acpi/arm64/iort.c
>> + * => commit ca78d3173cff3503bcd15723b049757f75762d15
>> + *
>> + * Xen modification:
>> + * Sameer Goel 
>> + * Copyright (C) 2017, The Linux Foundation, All rights reserved.
>> + *
>>    */
>>   -#define pr_fmt(fmt)    "ACPI: IORT: " fmt
>> -
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +/* Xen: Define compatibility functions */
>> +#define FW_BUG    "[Firmware Bug]: "
>> +#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
>> +#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
>> +
>> +/* Alias to Xen allocation helpers */
>> +#define kfree xfree
>> +#define kmalloc(size, flags)    _xmalloc(size, sizeof(void *))
>> +#define kzalloc(size, flags)    _xzalloc(size, sizeof(void *))
>> +
>> +/* Redefine WARN macros */
>> +#undef WARN
>> +#undef WARN_ON
>> +#define WARN(condition, format...) ({    \
>> +    int __ret_warn_on = !!(condition);    \
>> +    if

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-11-08 Thread Manish Jaggi 

Hi Sameer

On 9/21/2017 6:07 AM, Sameer Goel wrote:

Add support for parsing IORT table to initialize SMMU devices.
* The code for creating an SMMU device has been modified, so that the SMMU
device can be initialized.
* The NAMED NODE code has been commented out as this will need DOM0 kernel
support.
* ITS code has been included but it has not been tested.

Signed-off-by: Sameer Goel 
Followup of the discussions we had on iort parsing and querying streamID 
and deviceId based on RID.

I have extended your patchset with a patch that provides an alternative
way of parsing iort into maps : {rid-streamid}, {rid-deviceID)
which can directly be looked up for searching streamId for a rid. This
will remove the need to traverse iort table again.

The test patch just describes the proposed flow and how the parsing and
query code might fit in. I have not tested it.
The code only compiles.

https://github.com/mjaggi-cavium/xen-wip/commit/df006d64bdbb5c8344de5a710da8bf64c9e8edd5
(This repo has all 7 of your patches + test code patch merged.

Note: The commit text of the patch describes the basic flow /assumptions 
/ usage of functions.

Please see the code along with the v2 design draft.
[RFC] [Draft Design v2] ACPI/IORT Support in Xen.
https://lists.xen.org/archives/html/xen-devel/2017-11/msg00512.html

I seek your advice on this. Please provide your feedback.

Thanks
Manish



---
  xen/arch/arm/setup.c   |   3 +
  xen/drivers/acpi/Makefile  |   1 +
  xen/drivers/acpi/arm/Makefile  |   1 +
  xen/drivers/acpi/arm/iort.c| 173 +
  xen/drivers/passthrough/arm/smmu.c |   1 +
  xen/include/acpi/acpi_iort.h   |  17 ++--
  xen/include/asm-arm/device.h   |   2 +
  xen/include/xen/acpi.h |  21 +
  xen/include/xen/pci.h  |   8 ++
  9 files changed, 146 insertions(+), 81 deletions(-)
  create mode 100644 xen/drivers/acpi/arm/Makefile

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 92f173b..4ba09b2 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -49,6 +49,7 @@
  #include 
  #include 
  #include 
+#include 
  
  struct bootinfo __initdata bootinfo;
  
@@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
  
  tasklet_subsys_init();
  
+/* Parse the ACPI iort data */

+acpi_iort_init();
  
  xsm_dt_init();
  
diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile

index 444b11d..e7ffd82 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -1,5 +1,6 @@
  subdir-y += tables
  subdir-y += utilities
+subdir-$(CONFIG_ARM) += arm
  subdir-$(CONFIG_X86) += apei
  
  obj-bin-y += tables.init.o

diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
new file mode 100644
index 000..7c039bb
--- /dev/null
+++ b/xen/drivers/acpi/arm/Makefile
@@ -0,0 +1 @@
+obj-y += iort.o
diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
index 2e368a6..7f54062 100644
--- a/xen/drivers/acpi/arm/iort.c
+++ b/xen/drivers/acpi/arm/iort.c
@@ -14,17 +14,47 @@
   * This file implements early detection/parsing of I/O mapping
   * reported to OS through firmware via I/O Remapping Table (IORT)
   * IORT document number: ARM DEN 0049A
+ *
+ * Based on Linux drivers/acpi/arm64/iort.c
+ * => commit ca78d3173cff3503bcd15723b049757f75762d15
+ *
+ * Xen modification:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
   */
  
-#define pr_fmt(fmt)	"ACPI: IORT: " fmt

-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* Xen: Define compatibility functions */
+#define FW_BUG "[Firmware Bug]: "
+#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
+#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
+
+/* Alias to Xen allocation helpers */
+#define kfree xfree
+#define kmalloc(size, flags)_xmalloc(size, sizeof(void *))
+#define kzalloc(size, flags)_xzalloc(size, sizeof(void *))
+
+/* Redefine WARN macros */
+#undef WARN
+#undef WARN_ON
+#define WARN(condition, format...) ({  \
+   int __ret_warn_on = !!(condition);  \
+   if (unlikely(__ret_warn_on))\
+   printk(format); \
+   unlikely(__ret_warn_on);\
+})
+#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
+#define WARN_ON(cond)  (!!cond)
  
  #define IORT_TYPE_MASK(type)	(1 << (type))

  #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
@@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
acpi_status status;
  
  	if (node->type ==

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-10-24 Thread Julien Grall 

Hi Sameer,

On 19/10/17 16:21, Goel, Sameer wrote:

On 10/12/2017 8:06 AM, Julien Grall wrote:

+
+/* Redefine WARN macros */
+#undef WARN
+#undef WARN_ON
+#define WARN(condition, format...) ({    \
+    int __ret_warn_on = !!(condition);    \
+    if (unlikely(__ret_warn_on))    \
+    printk(format);    \
+    unlikely(__ret_warn_on);    \
+})


Again, you should at least try to modify the common code version before 
deciding to redefine it here.

The xen macro seems such that it explicitly tries to block a return by wrapping 
this macro in a loop. I had changed the common function in the last iteration 
and there seemed to be a pushback.


I don't think there was any pushback on changing the common code. Jan 
Beulich validly requested to move that change in a separate patch.





+#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
+#define WARN_ON(cond)  (!!cond)
     #define IORT_TYPE_MASK(type)    (1 << (type))
   #define IORT_MSI_TYPE    (1 << ACPI_IORT_NODE_ITS_GROUP)
@@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
   acpi_status status;
     if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
+    status = AE_NOT_IMPLEMENTED;
+/*
+ * We need the namespace object name from dsdt to match the iort node, this


Please add a "Xen: TODO:" in front.

Ok.



+ * will need additions to the kernel xen bus notifiers.
+ * So, disabling the named node code till a proposal is approved.
+ */
+#if 0
   struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
   struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
   struct acpi_iort_named_component *ncomp;
@@ -275,11 +312,12 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
   status = !strcmp(ncomp->device_name, buf.pointer) ?
   AE_OK : AE_NOT_FOUND;
   acpi_os_free(buf.pointer);
+#endif
   } else if (node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX) {
   struct acpi_iort_root_complex *pci_rc;
-    struct pci_bus *bus;
+    struct pci_dev *pci_dev;


Do you really need to modify the code? Wouldn't it be possible to do

#define pci_bus pci_dev

The pci_dev is the container for the generic device. I wanted to call this out 
explicitly here. We can do the above if you insist :).


I agree this can be confusing to read. But that change is not justified 
for the goal you want to achieve. E.g importing the code from Linux and 
keep in sync in the future.


With a comment on top of the definitions, you could explain when pci_bus 
= pci_dev at the moment.


[...]


diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index 362d578..ad956d5 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -181,6 +181,7 @@ static void __iomem *devm_ioremap_resource(struct device 
*dev,
    * Xen: PCI functions
    * TODO: It should be implemented when PCI will be supported
    */
+#undef to_pci_dev


Why this change?

I had redefine the to_pci_dev to get the actual pci_dev struct. smmu driver 
does not use pci yet.


That should go in a separate commit in that case with probably all stub 
for PCI you added (see the pci.h).


The reason behind is those changes are not directly related to this 
patch. Patch should be kept simple and do one thing only. This makes 
easier to review.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-10-20 Thread Manish Jaggi 



On 10/19/2017 8:30 PM, Goel, Sameer wrote:

On 10/10/2017 6:36 AM, Manish Jaggi wrote:

Hi Sameer,
On 9/21/2017 6:07 AM, Sameer Goel wrote:

Add support for parsing IORT table to initialize SMMU devices.
* The code for creating an SMMU device has been modified, so that the SMMU
device can be initialized.
* The NAMED NODE code has been commented out as this will need DOM0 kernel
support.
* ITS code has been included but it has not been tested.

Could you please refactor this patch into another set of two patches.
I am planning to rebase my IORT for Dom0 Hiding patch rework on this patch.

I will try to break this up. Lets discuss this a bit more next week.

Please have a look at the draft design. [1]
[1] https://www.mail-archive.com/xen-devel@lists.xen.org/msg125951.html

Thanks,
Manish

Signed-off-by: Sameer Goel 
---
   xen/arch/arm/setup.c   |   3 +
   xen/drivers/acpi/Makefile  |   1 +
   xen/drivers/acpi/arm/Makefile  |   1 +
   xen/drivers/acpi/arm/iort.c| 173 
+
   xen/drivers/passthrough/arm/smmu.c |   1 +
   xen/include/acpi/acpi_iort.h   |  17 ++--
   xen/include/asm-arm/device.h   |   2 +
   xen/include/xen/acpi.h |  21 +
   xen/include/xen/pci.h  |   8 ++
   9 files changed, 146 insertions(+), 81 deletions(-)
   create mode 100644 xen/drivers/acpi/arm/Makefile

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 92f173b..4ba09b2 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -49,6 +49,7 @@
   #include 
   #include 
   #include 
+#include 
 struct bootinfo __initdata bootinfo;
   @@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
 tasklet_subsys_init();
   +/* Parse the ACPI iort data */
+acpi_iort_init();
 xsm_dt_init();
   diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
index 444b11d..e7ffd82 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -1,5 +1,6 @@
   subdir-y += tables
   subdir-y += utilities
+subdir-$(CONFIG_ARM) += arm
   subdir-$(CONFIG_X86) += apei
 obj-bin-y += tables.init.o
diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
new file mode 100644
index 000..7c039bb
--- /dev/null
+++ b/xen/drivers/acpi/arm/Makefile
@@ -0,0 +1 @@
+obj-y += iort.o
diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
index 2e368a6..7f54062 100644
--- a/xen/drivers/acpi/arm/iort.c
+++ b/xen/drivers/acpi/arm/iort.c
@@ -14,17 +14,47 @@
* This file implements early detection/parsing of I/O mapping
* reported to OS through firmware via I/O Remapping Table (IORT)
* IORT document number: ARM DEN 0049A
+ *
+ * Based on Linux drivers/acpi/arm64/iort.c
+ * => commit ca78d3173cff3503bcd15723b049757f75762d15
+ *
+ * Xen modification:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
*/
   -#define pr_fmt(fmt)"ACPI: IORT: " fmt
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* Xen: Define compatibility functions */
+#define FW_BUG"[Firmware Bug]: "
+#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
+#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
+
+/* Alias to Xen allocation helpers */
+#define kfree xfree
+#define kmalloc(size, flags)_xmalloc(size, sizeof(void *))
+#define kzalloc(size, flags)_xzalloc(size, sizeof(void *))
+
+/* Redefine WARN macros */
+#undef WARN
+#undef WARN_ON
+#define WARN(condition, format...) ({\
+int __ret_warn_on = !!(condition);\
+if (unlikely(__ret_warn_on))\
+printk(format);\
+unlikely(__ret_warn_on);\
+})
+#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
+#define WARN_ON(cond)  (!!cond)
 #define IORT_TYPE_MASK(type)(1 << (type))
   #define IORT_MSI_TYPE(1 << ACPI_IORT_NODE_ITS_GROUP)
@@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
   acpi_status status;
 if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
+status = AE_NOT_IMPLEMENTED;
+/*
+ * We need the namespace object name from dsdt to match the iort node, this
+ * will need additions to the kernel xen bus notifiers.
+ * So, disabling the named node code till a proposal is approved.
+ */
+#if 0
   struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
   struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
   struct acpi_iort_named_component *ncomp;
@@ -275,11 +312,12 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
   status = !strcmp(ncomp->device_name, buf.pointer)

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-10-19 Thread Goel, Sameer 


On 10/12/2017 8:06 AM, Julien Grall wrote:
> Hi Sameer,
> 
> On 21/09/17 01:37, Sameer Goel wrote:
>> Add support for parsing IORT table to initialize SMMU devices.
>> * The code for creating an SMMU device has been modified, so that the SMMU
>> device can be initialized.
>> * The NAMED NODE code has been commented out as this will need DOM0 kernel
>> support.
>> * ITS code has been included but it has not been tested.
>>
>> Signed-off-by: Sameer Goel 
>> ---
>>   xen/arch/arm/setup.c   |   3 +
>>   xen/drivers/acpi/Makefile  |   1 +
>>   xen/drivers/acpi/arm/Makefile  |   1 +
>>   xen/drivers/acpi/arm/iort.c    | 173 
>> +
>>   xen/drivers/passthrough/arm/smmu.c |   1 +
>>   xen/include/acpi/acpi_iort.h   |  17 ++--
>>   xen/include/asm-arm/device.h   |   2 +
>>   xen/include/xen/acpi.h |  21 +
>>   xen/include/xen/pci.h  |   8 ++
>>   9 files changed, 146 insertions(+), 81 deletions(-)
>>   create mode 100644 xen/drivers/acpi/arm/Makefile
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 92f173b..4ba09b2 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -49,6 +49,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>     struct bootinfo __initdata bootinfo;
>>   @@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>>     tasklet_subsys_init();
>>   +    /* Parse the ACPI iort data */
>> +    acpi_iort_init();
>>     xsm_dt_init();
>>   diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
>> index 444b11d..e7ffd82 100644
>> --- a/xen/drivers/acpi/Makefile
>> +++ b/xen/drivers/acpi/Makefile
>> @@ -1,5 +1,6 @@
>>   subdir-y += tables
>>   subdir-y += utilities
>> +subdir-$(CONFIG_ARM) += arm
>>   subdir-$(CONFIG_X86) += apei
>>     obj-bin-y += tables.init.o
>> diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
>> new file mode 100644
>> index 000..7c039bb
>> --- /dev/null
>> +++ b/xen/drivers/acpi/arm/Makefile
>> @@ -0,0 +1 @@
>> +obj-y += iort.o
>> diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
>> index 2e368a6..7f54062 100644
>> --- a/xen/drivers/acpi/arm/iort.c
>> +++ b/xen/drivers/acpi/arm/iort.c
>> @@ -14,17 +14,47 @@
>>    * This file implements early detection/parsing of I/O mapping
>>    * reported to OS through firmware via I/O Remapping Table (IORT)
>>    * IORT document number: ARM DEN 0049A
>> + *
>> + * Based on Linux drivers/acpi/arm64/iort.c
>> + * => commit ca78d3173cff3503bcd15723b049757f75762d15
>> + *
>> + * Xen modification:
>> + * Sameer Goel 
>> + * Copyright (C) 2017, The Linux Foundation, All rights reserved.
>> + *
>>    */
>>   -#define pr_fmt(fmt)    "ACPI: IORT: " fmt
>> -
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> +#include 
>> +#include 
> 
> Why do you need to include there? Can't this be done after all the  ?
I was just clubbing the acpi includes. I can move this after all xen/
> 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +/* Xen: Define compatibility functions */
>> +#define FW_BUG    "[Firmware Bug]: "
>> +#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
>> +#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
>> +
>> +/* Alias to Xen allocation helpers */
>> +#define kfree xfree
>> +#define kmalloc(size, flags)    _xmalloc(size, sizeof(void *))
>> +#define kzalloc(size, flags)    _xzalloc(size, sizeof(void *))
> 
> Likely you would need the same macros in the SMMUv3 driver. Could we think of 
> a common headers implementing the Linux compat layer?
I agree that this will be a very useful. i'll try to propose something.
> 
>> +
>> +/* Redefine WARN macros */
>> +#undef WARN
>> +#undef WARN_ON
>> +#define WARN(condition, format...) ({    \
>> +    int __ret_warn_on = !!(condition);    \
>> +    if (unlikely(__ret_warn_on))    \
>> +    printk(format);    \
>> +    unlikely(__ret_warn_on);    \
>> +})
> 
> Again, you should at least try to modify the common code version before 
> deciding to redefine it here.
The xen macro seems such that it explicitly tries to block a return by wrapping 
this macro in a loop. I had changed the common function in the last iteration 
and there seemed to be a pushback. 
> 
>> +#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
>> +#define WARN_ON(cond)  (!!cond)
>>     #define IORT_TYPE_MASK(type)    (1 << (type))
>>   #define IORT_MSI_TYPE    (1 << ACPI_IORT_NODE_ITS_GROUP)
>> @@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct 
>> acpi_iort_node *node,
>>   acpi_status status;
>>     if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
>> +  

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-10-19 Thread Goel, Sameer 

On 10/10/2017 6:36 AM, Manish Jaggi wrote:
> Hi Sameer,
> On 9/21/2017 6:07 AM, Sameer Goel wrote:
>> Add support for parsing IORT table to initialize SMMU devices.
>> * The code for creating an SMMU device has been modified, so that the SMMU
>> device can be initialized.
>> * The NAMED NODE code has been commented out as this will need DOM0 kernel
>> support.
>> * ITS code has been included but it has not been tested.
> Could you please refactor this patch into another set of two patches.
> I am planning to rebase my IORT for Dom0 Hiding patch rework on this patch.

I will try to break this up. Lets discuss this a bit more next week.
> Thanks,
> Manish
>> Signed-off-by: Sameer Goel 
>> ---
>>   xen/arch/arm/setup.c   |   3 +
>>   xen/drivers/acpi/Makefile  |   1 +
>>   xen/drivers/acpi/arm/Makefile  |   1 +
>>   xen/drivers/acpi/arm/iort.c    | 173 
>> +
>>   xen/drivers/passthrough/arm/smmu.c |   1 +
>>   xen/include/acpi/acpi_iort.h   |  17 ++--
>>   xen/include/asm-arm/device.h   |   2 +
>>   xen/include/xen/acpi.h |  21 +
>>   xen/include/xen/pci.h  |   8 ++
>>   9 files changed, 146 insertions(+), 81 deletions(-)
>>   create mode 100644 xen/drivers/acpi/arm/Makefile
>>
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 92f173b..4ba09b2 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -49,6 +49,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>     struct bootinfo __initdata bootinfo;
>>   @@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>>     tasklet_subsys_init();
>>   +    /* Parse the ACPI iort data */
>> +    acpi_iort_init();
>>     xsm_dt_init();
>>   diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
>> index 444b11d..e7ffd82 100644
>> --- a/xen/drivers/acpi/Makefile
>> +++ b/xen/drivers/acpi/Makefile
>> @@ -1,5 +1,6 @@
>>   subdir-y += tables
>>   subdir-y += utilities
>> +subdir-$(CONFIG_ARM) += arm
>>   subdir-$(CONFIG_X86) += apei
>>     obj-bin-y += tables.init.o
>> diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
>> new file mode 100644
>> index 000..7c039bb
>> --- /dev/null
>> +++ b/xen/drivers/acpi/arm/Makefile
>> @@ -0,0 +1 @@
>> +obj-y += iort.o
>> diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
>> index 2e368a6..7f54062 100644
>> --- a/xen/drivers/acpi/arm/iort.c
>> +++ b/xen/drivers/acpi/arm/iort.c
>> @@ -14,17 +14,47 @@
>>    * This file implements early detection/parsing of I/O mapping
>>    * reported to OS through firmware via I/O Remapping Table (IORT)
>>    * IORT document number: ARM DEN 0049A
>> + *
>> + * Based on Linux drivers/acpi/arm64/iort.c
>> + * => commit ca78d3173cff3503bcd15723b049757f75762d15
>> + *
>> + * Xen modification:
>> + * Sameer Goel 
>> + * Copyright (C) 2017, The Linux Foundation, All rights reserved.
>> + *
>>    */
>>   -#define pr_fmt(fmt)    "ACPI: IORT: " fmt
>> -
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> -#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +
>> +/* Xen: Define compatibility functions */
>> +#define FW_BUG    "[Firmware Bug]: "
>> +#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
>> +#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
>> +
>> +/* Alias to Xen allocation helpers */
>> +#define kfree xfree
>> +#define kmalloc(size, flags)    _xmalloc(size, sizeof(void *))
>> +#define kzalloc(size, flags)    _xzalloc(size, sizeof(void *))
>> +
>> +/* Redefine WARN macros */
>> +#undef WARN
>> +#undef WARN_ON
>> +#define WARN(condition, format...) ({    \
>> +    int __ret_warn_on = !!(condition);    \
>> +    if (unlikely(__ret_warn_on))    \
>> +    printk(format);    \
>> +    unlikely(__ret_warn_on);    \
>> +})
>> +#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
>> +#define WARN_ON(cond)  (!!cond)
>>     #define IORT_TYPE_MASK(type)    (1 << (type))
>>   #define IORT_MSI_TYPE    (1 << ACPI_IORT_NODE_ITS_GROUP)
>> @@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct 
>> acpi_iort_node *node,
>>   acpi_status status;
>>     if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
>> +    status = AE_NOT_IMPLEMENTED;
>> +/*
>> + * We need the namespace object name from dsdt to match the iort node, this
>> + * will need additions to the kernel xen bus notifiers.
>> + * So, disabling the named node code till a proposal is approved.
>> + */
>> +#if 0
>>   struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
>>   struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
>>   struct

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-10-12 Thread Julien Grall 

Hi Sameer,

On 21/09/17 01:37, Sameer Goel wrote:

@@ -583,14 +631,13 @@ const struct iommu_ops *iort_iommu_configure(struct 
device *dev)
u32 streamid = 0;
  
  	if (dev_is_pci(dev)) {

-   struct pci_bus *bus = to_pci_dev(dev)->bus;
+   struct pci_dev *pci_device = to_pci_dev(dev);
u32 rid;
  
-		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,

-  );
+   rid = PCI_BDF2(pci_device->bus, pci_device->devfn);


I forgot to answer on this bit. On the previous it was mentioned this 
was wrong, but still there.


Whilst I can understand that implementing pci_for_each_dma_alias could 
require some work in Xen, I don't want to see rid = PCI_BDF2(...) 
spreading the code. So what's the plan?


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-10-12 Thread Julien Grall 

Hi Sameer,

On 21/09/17 01:37, Sameer Goel wrote:

Add support for parsing IORT table to initialize SMMU devices.
* The code for creating an SMMU device has been modified, so that the SMMU
device can be initialized.
* The NAMED NODE code has been commented out as this will need DOM0 kernel
support.
* ITS code has been included but it has not been tested.

Signed-off-by: Sameer Goel 
---
  xen/arch/arm/setup.c   |   3 +
  xen/drivers/acpi/Makefile  |   1 +
  xen/drivers/acpi/arm/Makefile  |   1 +
  xen/drivers/acpi/arm/iort.c| 173 +
  xen/drivers/passthrough/arm/smmu.c |   1 +
  xen/include/acpi/acpi_iort.h   |  17 ++--
  xen/include/asm-arm/device.h   |   2 +
  xen/include/xen/acpi.h |  21 +
  xen/include/xen/pci.h  |   8 ++
  9 files changed, 146 insertions(+), 81 deletions(-)
  create mode 100644 xen/drivers/acpi/arm/Makefile

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 92f173b..4ba09b2 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -49,6 +49,7 @@
  #include 
  #include 
  #include 
+#include 
  
  struct bootinfo __initdata bootinfo;
  
@@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
  
  tasklet_subsys_init();
  
+/* Parse the ACPI iort data */

+acpi_iort_init();
  
  xsm_dt_init();
  
diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile

index 444b11d..e7ffd82 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -1,5 +1,6 @@
  subdir-y += tables
  subdir-y += utilities
+subdir-$(CONFIG_ARM) += arm
  subdir-$(CONFIG_X86) += apei
  
  obj-bin-y += tables.init.o

diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
new file mode 100644
index 000..7c039bb
--- /dev/null
+++ b/xen/drivers/acpi/arm/Makefile
@@ -0,0 +1 @@
+obj-y += iort.o
diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
index 2e368a6..7f54062 100644
--- a/xen/drivers/acpi/arm/iort.c
+++ b/xen/drivers/acpi/arm/iort.c
@@ -14,17 +14,47 @@
   * This file implements early detection/parsing of I/O mapping
   * reported to OS through firmware via I/O Remapping Table (IORT)
   * IORT document number: ARM DEN 0049A
+ *
+ * Based on Linux drivers/acpi/arm64/iort.c
+ * => commit ca78d3173cff3503bcd15723b049757f75762d15
+ *
+ * Xen modification:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
   */
  
-#define pr_fmt(fmt)	"ACPI: IORT: " fmt

-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
+#include 


Why do you need to include there? Can't this be done after all the  ?


+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* Xen: Define compatibility functions */
+#define FW_BUG "[Firmware Bug]: "
+#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
+#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
+
+/* Alias to Xen allocation helpers */
+#define kfree xfree
+#define kmalloc(size, flags)_xmalloc(size, sizeof(void *))
+#define kzalloc(size, flags)_xzalloc(size, sizeof(void *))


Likely you would need the same macros in the SMMUv3 driver. Could we 
think of a common headers implementing the Linux compat layer?



+
+/* Redefine WARN macros */
+#undef WARN
+#undef WARN_ON
+#define WARN(condition, format...) ({  \
+   int __ret_warn_on = !!(condition);  \
+   if (unlikely(__ret_warn_on))\
+   printk(format); \
+   unlikely(__ret_warn_on);\
+})


Again, you should at least try to modify the common code version before 
deciding to redefine it here.



+#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
+#define WARN_ON(cond)  (!!cond)
  
  #define IORT_TYPE_MASK(type)	(1 << (type))

  #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
@@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
acpi_status status;
  
  	if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {

+   status = AE_NOT_IMPLEMENTED;
+/*
+ * We need the namespace object name from dsdt to match the iort node, this


Please add a "Xen: TODO:" in front.


+ * will need additions to the kernel xen bus notifiers.
+ * So, disabling the named node code till a proposal is approved.
+ */
+#if 0
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
struct acpi_iort_named_component *ncomp;
@@ -275,11 +312,12 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
status =

Re: [Xen-devel] [RFC v2 5/7] acpi:arm64: Add support for parsing IORT table

2017-10-10 Thread Manish Jaggi 

Hi Sameer,
On 9/21/2017 6:07 AM, Sameer Goel wrote:

Add support for parsing IORT table to initialize SMMU devices.
* The code for creating an SMMU device has been modified, so that the SMMU
device can be initialized.
* The NAMED NODE code has been commented out as this will need DOM0 kernel
support.
* ITS code has been included but it has not been tested.

Could you please refactor this patch into another set of two patches.
I am planning to rebase my IORT for Dom0 Hiding patch rework on this patch.
Thanks,
Manish

Signed-off-by: Sameer Goel 
---
  xen/arch/arm/setup.c   |   3 +
  xen/drivers/acpi/Makefile  |   1 +
  xen/drivers/acpi/arm/Makefile  |   1 +
  xen/drivers/acpi/arm/iort.c| 173 +
  xen/drivers/passthrough/arm/smmu.c |   1 +
  xen/include/acpi/acpi_iort.h   |  17 ++--
  xen/include/asm-arm/device.h   |   2 +
  xen/include/xen/acpi.h |  21 +
  xen/include/xen/pci.h  |   8 ++
  9 files changed, 146 insertions(+), 81 deletions(-)
  create mode 100644 xen/drivers/acpi/arm/Makefile

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 92f173b..4ba09b2 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -49,6 +49,7 @@
  #include 
  #include 
  #include 
+#include 
  
  struct bootinfo __initdata bootinfo;
  
@@ -796,6 +797,8 @@ void __init start_xen(unsigned long boot_phys_offset,
  
  tasklet_subsys_init();
  
+/* Parse the ACPI iort data */

+acpi_iort_init();
  
  xsm_dt_init();
  
diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile

index 444b11d..e7ffd82 100644
--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -1,5 +1,6 @@
  subdir-y += tables
  subdir-y += utilities
+subdir-$(CONFIG_ARM) += arm
  subdir-$(CONFIG_X86) += apei
  
  obj-bin-y += tables.init.o

diff --git a/xen/drivers/acpi/arm/Makefile b/xen/drivers/acpi/arm/Makefile
new file mode 100644
index 000..7c039bb
--- /dev/null
+++ b/xen/drivers/acpi/arm/Makefile
@@ -0,0 +1 @@
+obj-y += iort.o
diff --git a/xen/drivers/acpi/arm/iort.c b/xen/drivers/acpi/arm/iort.c
index 2e368a6..7f54062 100644
--- a/xen/drivers/acpi/arm/iort.c
+++ b/xen/drivers/acpi/arm/iort.c
@@ -14,17 +14,47 @@
   * This file implements early detection/parsing of I/O mapping
   * reported to OS through firmware via I/O Remapping Table (IORT)
   * IORT document number: ARM DEN 0049A
+ *
+ * Based on Linux drivers/acpi/arm64/iort.c
+ * => commit ca78d3173cff3503bcd15723b049757f75762d15
+ *
+ * Xen modification:
+ * Sameer Goel 
+ * Copyright (C) 2017, The Linux Foundation, All rights reserved.
+ *
   */
  
-#define pr_fmt(fmt)	"ACPI: IORT: " fmt

-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/* Xen: Define compatibility functions */
+#define FW_BUG "[Firmware Bug]: "
+#define pr_err(fmt, ...) printk(XENLOG_ERR fmt, ## __VA_ARGS__)
+#define pr_warn(fmt, ...) printk(XENLOG_WARNING fmt, ## __VA_ARGS__)
+
+/* Alias to Xen allocation helpers */
+#define kfree xfree
+#define kmalloc(size, flags)_xmalloc(size, sizeof(void *))
+#define kzalloc(size, flags)_xzalloc(size, sizeof(void *))
+
+/* Redefine WARN macros */
+#undef WARN
+#undef WARN_ON
+#define WARN(condition, format...) ({  \
+   int __ret_warn_on = !!(condition);  \
+   if (unlikely(__ret_warn_on))\
+   printk(format); \
+   unlikely(__ret_warn_on);\
+})
+#define WARN_TAINT(cond, taint, format...) WARN(cond, format)
+#define WARN_ON(cond)  (!!cond)
  
  #define IORT_TYPE_MASK(type)	(1 << (type))

  #define IORT_MSI_TYPE (1 << ACPI_IORT_NODE_ITS_GROUP)
@@ -256,6 +286,13 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
acpi_status status;
  
  	if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {

+   status = AE_NOT_IMPLEMENTED;
+/*
+ * We need the namespace object name from dsdt to match the iort node, this
+ * will need additions to the kernel xen bus notifiers.
+ * So, disabling the named node code till a proposal is approved.
+ */
+#if 0
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
struct acpi_iort_named_component *ncomp;
@@ -275,11 +312,12 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
status = !strcmp(ncomp->device_name, buf.pointer) ?
AE_OK : AE_NOT_FOUND;
acpi_os_free(buf.pointer);
+#endif
} else if (node->type ==