Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
Hi,
On 11/08/17 05:54, Manish Jaggi wrote:
On 8/10/2017 6:44 PM, Julien Grall wrote:
On 08/10/2017 02:00 PM, Manish Jaggi wrote:
HI Julien,
On 8/10/2017 5:43 PM, Julien Grall wrote:
On 10/08/17 13:00, Manish Jaggi wrote:
Hi Julien,
On 8/10/2017 4:58 PM, Julien Grall wrote:
On 10/08/17 12:21, Manish Jaggi wrote:
Hi Julien,
On 6/21/2017 6:53 PM, Julien Grall wrote:
Hi Manish,
On 21/06/17 02:01, Manish Jaggi wrote:
This patch series adds the support of ITS for ACPI hardware
domain.
It is tested on staging branch with has ITS v12 patchset by Andre.
I have tried to incorporate the review comments on the RFC v1/v2
patch.
The single patch in RFC is now split into 4 patches.
I will comment here rather than on each patches.
Patch1: ARM: ITS: Add translation_id to host_its
Adds translation_id in host_its data structure, which is
populated
from
translation_id read from firmwar MADT. This value is then
programmed
into
local MADT created for hardware domain in patch 4.
I don't see any reason to store value that will only be used for
generating the MADT which BTW is just a copy for the ITS.
Instead we
should copy over the MADT entries.
There are two approaches,
If I use the standard API acpi_table_parse_madt which would iterate
over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to
maintain the
addr and translation_id in some data structure, to be filled
later in
the hwdomain copy of madt generic translator.
If I don't use the standard API I have to add code to manually
parse all
the translator entries.
There are a 3rd approach I suggested and ignored... The ITS entries
for Dom0 is exactly the same as the host entries.
Yes, and if not passed properly dom0 wont get device interrupts...
So you only need to do a verbatim copy of the entry...
Can you please check patch 4/2, the translation_id and address are
passed verbatim, the other values are reserved in
acpi_madt_generic_translator.
For ACPI, we took the approach to only rewrite what's necessary and
give the rest to Dom0 as it is. If newer version of ACPI re-used
those fields, then they will be copied over to Dom0. I don't
consider it as an issue because the problem would be the same if
those fields have an important meaning for the platform.
Few thoughts...
If we follow this approach, few points needs to be considered
- If ACPI may use the reserved information later it could be equally
important for dom0 and Xen,
so it might be useful to keep reserved in xen as well.
I already covered that in my previous e-mail.
Yes, I am just stating it again for xen.
- For platforms which use dt, translation_id is not required to be
stored in struct host_its, similarly for platforms which use acpi
dt_node pointer might be of no use.
So we can have struct host_its having a union with dt_device_node *
for dt and acpi_madt_generic_translator * for acpi.
IMHO this could be an approach we can take.
struct host_its {
struct list_head entry;
-const struct dt_device_node *dt_node;
+ union {
+const struct dt_device_node *dt_node;
+const struct acpi_madt_generic_translator *acpi_its_entry;
+};
paddr_t addr;
What don't you get in my previous e-mail? A no is a no, full stop.
This is not helping.
Just do what we do in *_make_hwdom_madt. That will work here with no
need of a union or anything else.
The patchset provides two features
(a) populates host_its list from ACPI tables, so ACPI xen can use ITS
(b) provides a MADT with ITS information to dom0.
What I am focusing with union is for (a) ,
and (b) code would be simpler if we use the union in (a).
You seem to be discounting (a) in comments so far.
why union? as I have mentioned before...
It will make the host_its structure accommodate dt node and
acpi_madt_generic_translator, both has same purpose.
If one is valid why not other.
You commented on "Even the DT code can be reworked to avoid storing the
node.", so I guess you can easily deduce by yourself why I don't think
it should be used.
To repeat for the 4th time, there are need to keep around both DT and
ACPI pointer in the structure. Maybe some code will help you to understand:
for (i = 0; i < nr_its; i++)
{
struct acpi_madt_ *entry;
entry = acpi_table_get_entry_madt(ACPI_MADT_TYPE_ITS, i);
BUG_ON(entry);
ACPI_MEMCPY(..., ...);
}
Job done.
please provide a technical reason for not doing it.
I would appreciate to not repeat 4 times (including on IRC) the same
things... I don't have spare time for that. So as I said either you
address my comment or I am going to ignore this until I find spare time
to deal with it.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
On 8/10/2017 6:44 PM, Julien Grall wrote:
On 08/10/2017 02:00 PM, Manish Jaggi wrote:
HI Julien,
On 8/10/2017 5:43 PM, Julien Grall wrote:
On 10/08/17 13:00, Manish Jaggi wrote:
Hi Julien,
On 8/10/2017 4:58 PM, Julien Grall wrote:
On 10/08/17 12:21, Manish Jaggi wrote:
Hi Julien,
On 6/21/2017 6:53 PM, Julien Grall wrote:
Hi Manish,
On 21/06/17 02:01, Manish Jaggi wrote:
This patch series adds the support of ITS for ACPI hardware
domain.
It is tested on staging branch with has ITS v12 patchset by Andre.
I have tried to incorporate the review comments on the RFC v1/v2
patch.
The single patch in RFC is now split into 4 patches.
I will comment here rather than on each patches.
Patch1: ARM: ITS: Add translation_id to host_its
Adds translation_id in host_its data structure, which is
populated
from
translation_id read from firmwar MADT. This value is then
programmed
into
local MADT created for hardware domain in patch 4.
I don't see any reason to store value that will only be used for
generating the MADT which BTW is just a copy for the ITS.
Instead we
should copy over the MADT entries.
There are two approaches,
If I use the standard API acpi_table_parse_madt which would iterate
over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to
maintain the
addr and translation_id in some data structure, to be filled
later in
the hwdomain copy of madt generic translator.
If I don't use the standard API I have to add code to manually
parse all
the translator entries.
There are a 3rd approach I suggested and ignored... The ITS entries
for Dom0 is exactly the same as the host entries.
Yes, and if not passed properly dom0 wont get device interrupts...
So you only need to do a verbatim copy of the entry...
Can you please check patch 4/2, the translation_id and address are
passed verbatim, the other values are reserved in
acpi_madt_generic_translator.
For ACPI, we took the approach to only rewrite what's necessary and
give the rest to Dom0 as it is. If newer version of ACPI re-used
those fields, then they will be copied over to Dom0. I don't
consider it as an issue because the problem would be the same if
those fields have an important meaning for the platform.
Few thoughts...
If we follow this approach, few points needs to be considered
- If ACPI may use the reserved information later it could be equally
important for dom0 and Xen,
so it might be useful to keep reserved in xen as well.
I already covered that in my previous e-mail.
Yes, I am just stating it again for xen.
- For platforms which use dt, translation_id is not required to be
stored in struct host_its, similarly for platforms which use acpi
dt_node pointer might be of no use.
So we can have struct host_its having a union with dt_device_node *
for dt and acpi_madt_generic_translator * for acpi.
IMHO this could be an approach we can take.
struct host_its {
struct list_head entry;
-const struct dt_device_node *dt_node;
+ union {
+const struct dt_device_node *dt_node;
+const struct acpi_madt_generic_translator *acpi_its_entry;
+};
paddr_t addr;
What don't you get in my previous e-mail? A no is a no, full stop.
This is not helping.
Just do what we do in *_make_hwdom_madt. That will work here with no
need of a union or anything else.
The patchset provides two features
(a) populates host_its list from ACPI tables, so ACPI xen can use ITS
(b) provides a MADT with ITS information to dom0.
What I am focusing with union is for (a) ,
and (b) code would be simpler if we use the union in (a).
You seem to be discounting (a) in comments so far.
why union? as I have mentioned before...
It will make the host_its structure accommodate dt node and
acpi_madt_generic_translator, both has same purpose.
If one is valid why not other.
please provide a technical reason for not doing it.
Even the DT code can be reworked to avoid storing the node.
we can have a separate patch for that.
Cheers,
Cheers!
Sending next rev shortly.
-manish
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
On 08/10/2017 02:00 PM, Manish Jaggi wrote:
HI Julien,
On 8/10/2017 5:43 PM, Julien Grall wrote:
On 10/08/17 13:00, Manish Jaggi wrote:
Hi Julien,
On 8/10/2017 4:58 PM, Julien Grall wrote:
On 10/08/17 12:21, Manish Jaggi wrote:
Hi Julien,
On 6/21/2017 6:53 PM, Julien Grall wrote:
Hi Manish,
On 21/06/17 02:01, Manish Jaggi wrote:
This patch series adds the support of ITS for ACPI hardware domain.
It is tested on staging branch with has ITS v12 patchset by Andre.
I have tried to incorporate the review comments on the RFC v1/v2
patch.
The single patch in RFC is now split into 4 patches.
I will comment here rather than on each patches.
Patch1: ARM: ITS: Add translation_id to host_its
Adds translation_id in host_its data structure, which is populated
from
translation_id read from firmwar MADT. This value is then
programmed
into
local MADT created for hardware domain in patch 4.
I don't see any reason to store value that will only be used for
generating the MADT which BTW is just a copy for the ITS. Instead we
should copy over the MADT entries.
There are two approaches,
If I use the standard API acpi_table_parse_madt which would iterate
over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to maintain the
addr and translation_id in some data structure, to be filled later in
the hwdomain copy of madt generic translator.
If I don't use the standard API I have to add code to manually
parse all
the translator entries.
There are a 3rd approach I suggested and ignored... The ITS entries
for Dom0 is exactly the same as the host entries.
Yes, and if not passed properly dom0 wont get device interrupts...
So you only need to do a verbatim copy of the entry...
Can you please check patch 4/2, the translation_id and address are
passed verbatim, the other values are reserved in
acpi_madt_generic_translator.
For ACPI, we took the approach to only rewrite what's necessary and
give the rest to Dom0 as it is. If newer version of ACPI re-used those
fields, then they will be copied over to Dom0. I don't consider it as
an issue because the problem would be the same if those fields have an
important meaning for the platform.
Few thoughts...
If we follow this approach, few points needs to be considered
- If ACPI may use the reserved information later it could be equally
important for dom0 and Xen,
so it might be useful to keep reserved in xen as well.
I already covered that in my previous e-mail.
- For platforms which use dt, translation_id is not required to be
stored in struct host_its, similarly for platforms which use acpi
dt_node pointer might be of no use.
So we can have struct host_its having a union with dt_device_node * for
dt and acpi_madt_generic_translator * for acpi.
IMHO this could be an approach we can take.
struct host_its {
struct list_head entry;
-const struct dt_device_node *dt_node;
+ union {
+const struct dt_device_node *dt_node;
+const struct acpi_madt_generic_translator *acpi_its_entry;
+};
paddr_t addr;
What don't you get in my previous e-mail? A no is a no, full stop.
Just do what we do in *_make_hwdom_madt. That will work here with no
need of a union or anything else. Even the DT code can be reworked to
avoid storing the node.
We could even have the common code go through the MADT entry one by one
and let the specific driver updating what's necessary avoid ACPI_MEMCPY
duplication in each bit. But this is a longer term thoughts than for.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
HI Julien,
On 8/10/2017 5:43 PM, Julien Grall wrote:
On 10/08/17 13:00, Manish Jaggi wrote:
Hi Julien,
On 8/10/2017 4:58 PM, Julien Grall wrote:
On 10/08/17 12:21, Manish Jaggi wrote:
Hi Julien,
On 6/21/2017 6:53 PM, Julien Grall wrote:
Hi Manish,
On 21/06/17 02:01, Manish Jaggi wrote:
This patch series adds the support of ITS for ACPI hardware domain.
It is tested on staging branch with has ITS v12 patchset by Andre.
I have tried to incorporate the review comments on the RFC v1/v2
patch.
The single patch in RFC is now split into 4 patches.
I will comment here rather than on each patches.
Patch1: ARM: ITS: Add translation_id to host_its
Adds translation_id in host_its data structure, which is populated
from
translation_id read from firmwar MADT. This value is then
programmed
into
local MADT created for hardware domain in patch 4.
I don't see any reason to store value that will only be used for
generating the MADT which BTW is just a copy for the ITS. Instead we
should copy over the MADT entries.
There are two approaches,
If I use the standard API acpi_table_parse_madt which would iterate
over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to maintain the
addr and translation_id in some data structure, to be filled later in
the hwdomain copy of madt generic translator.
If I don't use the standard API I have to add code to manually
parse all
the translator entries.
There are a 3rd approach I suggested and ignored... The ITS entries
for Dom0 is exactly the same as the host entries.
Yes, and if not passed properly dom0 wont get device interrupts...
So you only need to do a verbatim copy of the entry...
Can you please check patch 4/2, the translation_id and address are
passed verbatim, the other values are reserved in
acpi_madt_generic_translator.
For ACPI, we took the approach to only rewrite what's necessary and
give the rest to Dom0 as it is. If newer version of ACPI re-used those
fields, then they will be copied over to Dom0. I don't consider it as
an issue because the problem would be the same if those fields have an
important meaning for the platform.
Few thoughts...
If we follow this approach, few points needs to be considered
- If ACPI may use the reserved information later it could be equally
important for dom0 and Xen,
so it might be useful to keep reserved in xen as well.
- For platforms which use dt, translation_id is not required to be
stored in struct host_its, similarly for platforms which use acpi
dt_node pointer might be of no use.
So we can have struct host_its having a union with dt_device_node * for
dt and acpi_madt_generic_translator * for acpi.
IMHO this could be an approach we can take.
struct host_its {
struct list_head entry;
-const struct dt_device_node *dt_node;
+ union {
+const struct dt_device_node *dt_node;
+const struct acpi_madt_generic_translator *acpi_its_entry;
+};
paddr_t addr;
Could you please detail 3rd approach and how different it is from
approach 2.
ACPI_MEMCPY(its, host_its, size);
Cheers,
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
On 10/08/17 13:00, Manish Jaggi wrote: Hi Julien, On 8/10/2017 4:58 PM, Julien Grall wrote: On 10/08/17 12:21, Manish Jaggi wrote: Hi Julien, On 6/21/2017 6:53 PM, Julien Grall wrote: Hi Manish, On 21/06/17 02:01, Manish Jaggi wrote: This patch series adds the support of ITS for ACPI hardware domain. It is tested on staging branch with has ITS v12 patchset by Andre. I have tried to incorporate the review comments on the RFC v1/v2 patch. The single patch in RFC is now split into 4 patches. I will comment here rather than on each patches. Patch1: ARM: ITS: Add translation_id to host_its Adds translation_id in host_its data structure, which is populated from translation_id read from firmwar MADT. This value is then programmed into local MADT created for hardware domain in patch 4. I don't see any reason to store value that will only be used for generating the MADT which BTW is just a copy for the ITS. Instead we should copy over the MADT entries. There are two approaches, If I use the standard API acpi_table_parse_madt which would iterate over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to maintain the addr and translation_id in some data structure, to be filled later in the hwdomain copy of madt generic translator. If I don't use the standard API I have to add code to manually parse all the translator entries. There are a 3rd approach I suggested and ignored... The ITS entries for Dom0 is exactly the same as the host entries. Yes, and if not passed properly dom0 wont get device interrupts... So you only need to do a verbatim copy of the entry... Can you please check patch 4/2, the translation_id and address are passed verbatim, the other values are reserved in acpi_madt_generic_translator. For ACPI, we took the approach to only rewrite what's necessary and give the rest to Dom0 as it is. If newer version of ACPI re-used those fields, then they will be copied over to Dom0. I don't consider it as an issue because the problem would be the same if those fields have an important meaning for the platform. Could you please detail 3rd approach and how different it is from approach 2. ACPI_MEMCPY(its, host_its, size); Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
Hi Julien,
On 8/10/2017 4:58 PM, Julien Grall wrote:
On 10/08/17 12:21, Manish Jaggi wrote:
Hi Julien,
On 6/21/2017 6:53 PM, Julien Grall wrote:
Hi Manish,
On 21/06/17 02:01, Manish Jaggi wrote:
This patch series adds the support of ITS for ACPI hardware domain.
It is tested on staging branch with has ITS v12 patchset by Andre.
I have tried to incorporate the review comments on the RFC v1/v2
patch.
The single patch in RFC is now split into 4 patches.
I will comment here rather than on each patches.
Patch1: ARM: ITS: Add translation_id to host_its
Adds translation_id in host_its data structure, which is populated
from
translation_id read from firmwar MADT. This value is then programmed
into
local MADT created for hardware domain in patch 4.
I don't see any reason to store value that will only be used for
generating the MADT which BTW is just a copy for the ITS. Instead we
should copy over the MADT entries.
There are two approaches,
If I use the standard API acpi_table_parse_madt which would iterate
over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to maintain the
addr and translation_id in some data structure, to be filled later in
the hwdomain copy of madt generic translator.
If I don't use the standard API I have to add code to manually parse all
the translator entries.
There are a 3rd approach I suggested and ignored... The ITS entries
for Dom0 is exactly the same as the host entries.
Yes, and if not passed properly dom0 wont get device interrupts...
So you only need to do a verbatim copy of the entry...
Can you please check patch 4/2, the translation_id and address are
passed verbatim, the other values are reserved in
acpi_madt_generic_translator.
Could you please detail 3rd approach and how different it is from
approach 2.
Which of the two you find cleaner?
This would also avoid to introduce a fake ID for DT as you currently
do in patch #2.
This can be avoided by storing translator_id only for acpi.
+static int add_to_host_its_list(u64 addr, u64 size,
+ u32 translation_id, const void *node)
+{
+struct host_its *its_data;
+its_data = xzalloc(struct host_its);
+
+if ( !its_data )
+return -1;
+
+if ( node )
+its_data->dt_node = node;
+else
+its_data->translation_id = translation_id;
+
+its_data->addr = addr;
+its_data->size = size;
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(&its_data->entry, &host_its_list);
+
+return 0;
What do you think?
I don't want to see the translation_id stored for no use at all but
creating the DOM0 ACPI tables. Is that clearer?
ok, I will remove it.
Cheers,
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
On 10/08/17 12:21, Manish Jaggi wrote:
Hi Julien,
On 6/21/2017 6:53 PM, Julien Grall wrote:
Hi Manish,
On 21/06/17 02:01, Manish Jaggi wrote:
This patch series adds the support of ITS for ACPI hardware domain.
It is tested on staging branch with has ITS v12 patchset by Andre.
I have tried to incorporate the review comments on the RFC v1/v2 patch.
The single patch in RFC is now split into 4 patches.
I will comment here rather than on each patches.
Patch1: ARM: ITS: Add translation_id to host_its
Adds translation_id in host_its data structure, which is populated from
translation_id read from firmwar MADT. This value is then programmed
into
local MADT created for hardware domain in patch 4.
I don't see any reason to store value that will only be used for
generating the MADT which BTW is just a copy for the ITS. Instead we
should copy over the MADT entries.
There are two approaches,
If I use the standard API acpi_table_parse_madt which would iterate
over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to maintain the
addr and translation_id in some data structure, to be filled later in
the hwdomain copy of madt generic translator.
If I don't use the standard API I have to add code to manually parse all
the translator entries.
There are a 3rd approach I suggested and ignored... The ITS entries for
Dom0 is exactly the same as the host entries. So you only need to do a
verbatim copy of the entry...
Which of the two you find cleaner?
This would also avoid to introduce a fake ID for DT as you currently
do in patch #2.
This can be avoided by storing translator_id only for acpi.
+static int add_to_host_its_list(u64 addr, u64 size,
+ u32 translation_id, const void *node)
+{
+struct host_its *its_data;
+its_data = xzalloc(struct host_its);
+
+if ( !its_data )
+return -1;
+
+if ( node )
+its_data->dt_node = node;
+else
+its_data->translation_id = translation_id;
+
+its_data->addr = addr;
+its_data->size = size;
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(&its_data->entry, &host_its_list);
+
+return 0;
What do you think?
I don't want to see the translation_id stored for no use at all but
creating the DOM0 ACPI tables. Is that clearer?
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
Hi Julien,
On 6/21/2017 6:53 PM, Julien Grall wrote:
Hi Manish,
On 21/06/17 02:01, Manish Jaggi wrote:
This patch series adds the support of ITS for ACPI hardware domain.
It is tested on staging branch with has ITS v12 patchset by Andre.
I have tried to incorporate the review comments on the RFC v1/v2 patch.
The single patch in RFC is now split into 4 patches.
I will comment here rather than on each patches.
Patch1: ARM: ITS: Add translation_id to host_its
Adds translation_id in host_its data structure, which is populated from
translation_id read from firmwar MADT. This value is then programmed
into
local MADT created for hardware domain in patch 4.
I don't see any reason to store value that will only be used for
generating the MADT which BTW is just a copy for the ITS. Instead we
should copy over the MADT entries.
There are two approaches,
If I use the standard API acpi_table_parse_madt which would iterate
over ACPI_MADT_TYPE_GENERIC_TRANSLATOR entries, I have to maintain the
addr and translation_id in some data structure, to be filled later in
the hwdomain copy of madt generic translator.
If I don't use the standard API I have to add code to manually parse all
the translator entries.
Which of the two you find cleaner?
This would also avoid to introduce a fake ID for DT as you currently
do in patch #2.
This can be avoided by storing translator_id only for acpi.
+static int add_to_host_its_list(u64 addr, u64 size,
+ u32 translation_id, const void *node)
+{
+struct host_its *its_data;
+its_data = xzalloc(struct host_its);
+
+if ( !its_data )
+return -1;
+
+if ( node )
+its_data->dt_node = node;
+else
+its_data->translation_id = translation_id;
+
+its_data->addr = addr;
+its_data->size = size;
+printk("GICv3: Found ITS @0x%lx\n", addr);
+
+list_add_tail(&its_data->entry, &host_its_list);
+
+return 0;
What do you think?
Patch2: ARM: ITS: ACPI: Introduce gicv3_its_acpi_init
Introduces function for its_acpi_init, which calls add_to_host_its_list
which is a common function also called from _dt variant.
Just reading at the description, there are a call for splitting this
patch... Looking at the code, you mix code movement and code addition.
Have a look at [1] to see how to break patches.
Yes I will break into multiple patches patch 2 and 4.
Patch3: ARM: ITS: Deny hardware domain access to its
Extends the gicv3_iomem_deny to include its regions as well
Patch4: ARM: ACPI: Add ITS to hardware domain MADT
This patch adds ITS information in hardware domain's MADT table.
Also this patch interoduces .get_hwdom_madt_size in gic_hw_operations,
to return the complete size of MADT table for hardware domain.
Same here.
Yes.
Manish Jaggi (4):
ARM: ITS: Add translation_id to host_its
ARM: ITS: ACPI: Introduce gicv3_its_acpi_init
ARM: ITS: Deny hardware domain access to its
ARM: ACPI: Add ITS to hardware domain MADT
xen/arch/arm/domain_build.c | 7 +--
xen/arch/arm/gic-v2.c| 6 +++
xen/arch/arm/gic-v3-its.c| 102
+++
xen/arch/arm/gic-v3.c| 31
xen/arch/arm/gic.c | 11 +
xen/include/asm-arm/gic.h| 3 ++
xen/include/asm-arm/gic_v3_its.h | 36 ++
7 files changed, 180 insertions(+), 16 deletions(-)
Cheers,
[1]
https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Making_good_patches
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] ARM: ACPI: ITS: Add ITS Support for ACPI hardware domain
Hi Manish, On 21/06/17 02:01, Manish Jaggi wrote: This patch series adds the support of ITS for ACPI hardware domain. It is tested on staging branch with has ITS v12 patchset by Andre. I have tried to incorporate the review comments on the RFC v1/v2 patch. The single patch in RFC is now split into 4 patches. I will comment here rather than on each patches. Patch1: ARM: ITS: Add translation_id to host_its Adds translation_id in host_its data structure, which is populated from translation_id read from firmwar MADT. This value is then programmed into local MADT created for hardware domain in patch 4. I don't see any reason to store value that will only be used for generating the MADT which BTW is just a copy for the ITS. Instead we should copy over the MADT entries. This would also avoid to introduce a fake ID for DT as you currently do in patch #2. Patch2: ARM: ITS: ACPI: Introduce gicv3_its_acpi_init Introduces function for its_acpi_init, which calls add_to_host_its_list which is a common function also called from _dt variant. Just reading at the description, there are a call for splitting this patch... Looking at the code, you mix code movement and code addition. Have a look at [1] to see how to break patches. Patch3: ARM: ITS: Deny hardware domain access to its Extends the gicv3_iomem_deny to include its regions as well Patch4: ARM: ACPI: Add ITS to hardware domain MADT This patch adds ITS information in hardware domain's MADT table. Also this patch interoduces .get_hwdom_madt_size in gic_hw_operations, to return the complete size of MADT table for hardware domain. Same here. Manish Jaggi (4): ARM: ITS: Add translation_id to host_its ARM: ITS: ACPI: Introduce gicv3_its_acpi_init ARM: ITS: Deny hardware domain access to its ARM: ACPI: Add ITS to hardware domain MADT xen/arch/arm/domain_build.c | 7 +-- xen/arch/arm/gic-v2.c| 6 +++ xen/arch/arm/gic-v3-its.c| 102 +++ xen/arch/arm/gic-v3.c| 31 xen/arch/arm/gic.c | 11 + xen/include/asm-arm/gic.h| 3 ++ xen/include/asm-arm/gic_v3_its.h | 36 ++ 7 files changed, 180 insertions(+), 16 deletions(-) Cheers, [1] https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Making_good_patches -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
