[PATCH v2 0/2] Port two ACPI fixes from Linux

[PATCH v2 0/2] Port two ACPI fixes from Linux

[Shannon Zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Len Brown (1):
  ACPI: disable ACPI cleanly when bad RSDP found

Tomasz Nowicki (1):
  ACPI/table: Always count matched and successfully parsed entries

 xen/drivers/acpi/tables.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

-- 
2.0.4

[PATCH v2 1/2] ACPI/table: Always count matched and successfully parsed entries

[Shannon Zhao]

From: Tomasz Nowicki <tomasz.nowicki@linaro.org>

Ported from Linux commit 4ceacd02f5a1795c5c697e0345ee10beef675290.

acpi_parse_entries() allows to traverse all available table entries (aka
subtables) by passing max_entries parameter equal to 0, but since its count
variable is only incremented if max_entries is not 0, the function always
returns 0 for max_entries equal to 0.  It would be more useful if it returned
the number of entries matched instead, so make it increment count in that
case too.

Acked-by: Grant Likely <grant.likely@linaro.org>
Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 xen/drivers/acpi/tables.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
index e57cf2a..1da2127 100644
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -239,10 +239,13 @@ acpi_table_parse_entries(char *id,
 		}
 
 		if (entry->type == entry_id
-		    && (!max_entries || count++ < max_entries))
+		    && (!max_entries || count < max_entries)) {
 			if (handler(entry, table_end))
 				return -EINVAL;
 
+			count++;
+		}
+
 		entry = (struct acpi_subtable_header *)
 		    ((unsigned long)entry + entry->length);
 	}
-- 
2.0.4

Re: [PATCH v2 1/2] ACPI/table: Always count matched and successfully parsed entries

[Julien Grall]

Hi Shannon,

On 14/08/15 04:36, Shannon Zhao wrote:
> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> 
> Ported from Linux commit 4ceacd02f5a1795c5c697e0345ee10beef675290.
> 
> acpi_parse_entries() allows to traverse all available table entries (aka
> subtables) by passing max_entries parameter equal to 0, but since its count
> variable is only incremented if max_entries is not 0, the function always
> returns 0 for max_entries equal to 0.  It would be more useful if it returned
> the number of entries matched instead, so make it increment count in that
> case too.
> 
> Acked-by: Grant Likely <grant.likely@linaro.org>
> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>

You may want to add an indentation to show this is the commit message
from Linux and not Xen.

One of the reason behind that it's Grant acked the patch for Linux and
not Xen. The patch is nearly the same this time (only the lines change),
but it may happen that we need to fix a bit.

Regards,

-- 
Julien Grall

Re: [PATCH v2 1/2] ACPI/table: Always count matched and successfully parsed entries

[Jan Beulich]

>>> On 14.08.15 at 12:50, <julien.grall@citrix.com> wrote:
> Hi Shannon,
> 
> On 14/08/15 04:36, Shannon Zhao wrote:
>> From: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>> 
>> Ported from Linux commit 4ceacd02f5a1795c5c697e0345ee10beef675290.
>> 
>> acpi_parse_entries() allows to traverse all available table entries (aka
>> subtables) by passing max_entries parameter equal to 0, but since its count
>> variable is only incremented if max_entries is not 0, the function always
>> returns 0 for max_entries equal to 0.  It would be more useful if it 
> returned
>> the number of entries matched instead, so make it increment count in that
>> case too.
>> 
>> Acked-by: Grant Likely <grant.likely@linaro.org>
>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> 
> You may want to add an indentation to show this is the commit message
> from Linux and not Xen.
> 
> One of the reason behind that it's Grant acked the patch for Linux and
> not Xen. The patch is nearly the same this time (only the lines change),
> but it may happen that we need to fix a bit.

Which is why I find it generally better to do

S-o-b:
S-o-b:
[Linux commit ...]
S-o-b:

(when no other comments need adding) which implicitly draws a line
between the Linux and Xen tags.

Jan

[PATCH v2 2/2] ACPI: disable ACPI cleanly when bad RSDP found

[Shannon Zhao]

From: Len Brown <len.brown@intel.com>

Ported from Linux commit 9e3a9d1ed8cc8db93e5c53e9a5b09065bd95de8b.

When ACPI is disabled in the BIOS of this VIA C3 box,
it invalidates the RSDP, which Linux notices:

ACPI Error (tbxfroot-0218): A valid RSDP was not found [20080926]

Bug Linux neglected to disable ACPI at that stage,
and later scribbled on smp_found_config:

ACPI: No APIC-table, disabling MPS

But this box doesn't run well in legacy PIC mode,
it needed IOAPIC mode to perform correctly:

http://lkml.org/lkml/2009/2/5/39

So exit ACPI mode cleanly when we first detect
that it is hopeless.

Signed-off-by: Len Brown <len.brown@intel.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 xen/drivers/acpi/tables.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
index 1da2127..6085fda 100644
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -329,7 +329,12 @@ static void __init check_multiple_madt(void)
 
 int __init acpi_table_init(void)
 {
-	acpi_initialize_tables(NULL, ACPI_MAX_TABLES, 0);
+	acpi_status status;
+
+	status = acpi_initialize_tables(NULL, ACPI_MAX_TABLES, 0);
+	if (ACPI_FAILURE(status))
+		return 1;
+
 	check_multiple_madt();
 	return 0;
 }
-- 
2.0.4

Re: [PATCH v2 0/2] Port two ACPI fixes from Linux

[Jan Beulich]

>>> On 14.08.15 at 05:36, <zhaoshenglong@huawei.com> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Len Brown (1):
>   ACPI: disable ACPI cleanly when bad RSDP found
> 
> Tomasz Nowicki (1):
>   ACPI/table: Always count matched and successfully parsed entries

These look good now, but is there a reason you didn't also port
the third commit I had referred you to (95df812dbd)?

Jan

Re: [PATCH v2 0/2] Port two ACPI fixes from Linux

[Shannon Zhao]

Hi Jan,

On 2015/8/14 18:24, Jan Beulich wrote:
>>>> On 14.08.15 at 05:36, <zhaoshenglong@huawei.com> wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Len Brown (1):
>>    ACPI: disable ACPI cleanly when bad RSDP found
>>
>> Tomasz Nowicki (1):
>>    ACPI/table: Always count matched and successfully parsed entries
>
> These look good now, but is there a reason you didn't also port
> the third commit I had referred you to (95df812dbd)?
>

Oh, sorry, forgot that. So this two patches could go first and port 
95df812dbd later or send a new version with 95df812dbd?

Thanks,
-- 
Shannon

Re: [PATCH v2 0/2] Port two ACPI fixes from Linux

[Jan Beulich]

>>> On 14.08.15 at 16:16, <shannon.zhao@linaro.org> wrote:
> On 2015/8/14 18:24, Jan Beulich wrote:
>>>>> On 14.08.15 at 05:36, <zhaoshenglong@huawei.com> wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Len Brown (1):
>>>    ACPI: disable ACPI cleanly when bad RSDP found
>>>
>>> Tomasz Nowicki (1):
>>>    ACPI/table: Always count matched and successfully parsed entries
>>
>> These look good now, but is there a reason you didn't also port
>> the third commit I had referred you to (95df812dbd)?
>>
> 
> Oh, sorry, forgot that. So this two patches could go first and port 
> 95df812dbd later or send a new version with 95df812dbd?

Since the patches will have to wait for the tree to re-open, there's
no big difference to me (I'd hope to have the 3rd patch from you
by the time they all could get committed).

Jan

Re: [PATCH v2 0/2] Port two ACPI fixes from Linux

[Shannon Zhao]

On 2015/8/14 22:29, Jan Beulich wrote:
>>>> On 14.08.15 at 16:16, <shannon.zhao@linaro.org> wrote:
>> On 2015/8/14 18:24, Jan Beulich wrote:
>>>>>> On 14.08.15 at 05:36, <zhaoshenglong@huawei.com> wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Len Brown (1):
>>>>     ACPI: disable ACPI cleanly when bad RSDP found
>>>>
>>>> Tomasz Nowicki (1):
>>>>     ACPI/table: Always count matched and successfully parsed entries
>>>
>>> These look good now, but is there a reason you didn't also port
>>> the third commit I had referred you to (95df812dbd)?
>>>
>>
>> Oh, sorry, forgot that. So this two patches could go first and port
>> 95df812dbd later or send a new version with 95df812dbd?
>
> Since the patches will have to wait for the tree to re-open, there's
> no big difference to me (I'd hope to have the 3rd patch from you
> by the time they all could get committed).
>
Ok, will send a new version.

Thanks,
-- 
Shannon