Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1327452 > unrolled thread

[PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen Shannon Zhao <zhaoshenglong@huawei.com> - 2016-02-05 04:20 +0100
    Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-06 11:30 +0100
      Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-02-08 12:00 +0100
        Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-08 23:30 +0100
          Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-02-09 12:20 +0100
            Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-10 23:30 +0100
              Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-02-11 17:10 +0100
                Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-11 23:30 +0100
                  Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2016-02-12 13:00 +0100
                    Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-12 14:00 +0100

#1327452 — [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen

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

ACPI 6.0 introduces a new table STAO to list the devices which are used
by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
UART is used by Xen. So here it hides UART from Dom0.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
CC: "Rafael J. Wysocki" <rjw@rjwysocki.net> (supporter:ACPI)
CC: Len Brown <lenb@kernel.org> (supporter:ACPI)
CC: linux-acpi@vger.kernel.org (open list:ACPI)
---
 drivers/acpi/bus.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 891c42d..5b5433d 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -46,6 +46,7 @@ ACPI_MODULE_NAME("bus");
 struct acpi_device *acpi_root;
 struct proc_dir_entry *acpi_root_dir;
 EXPORT_SYMBOL(acpi_root_dir);
+static u64 spcr_uart_addr;
 
 #ifdef CONFIG_X86
 #ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -105,6 +106,22 @@ acpi_status acpi_bus_get_status_handle(acpi_handle handle,
 	return status;
 }
 
+static bool acpi_check_device_is_ignored(acpi_handle handle)
+{
+	acpi_status status;
+	u64 addr;
+
+	/* Check if it should ignore the UART device */
+	if (spcr_uart_addr != 0) {
+		status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL,
+					       &addr);
+		if (ACPI_SUCCESS(status) && addr == spcr_uart_addr)
+			return true;
+	}
+
+	return false;
+}
+
 int acpi_bus_get_status(struct acpi_device *device)
 {
 	acpi_status status;
@@ -114,7 +131,8 @@ int acpi_bus_get_status(struct acpi_device *device)
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	acpi_set_device_status(device, sta);
+	if (!acpi_check_device_is_ignored(device->handle))
+		acpi_set_device_status(device, sta);
 
 	if (device->status.functional && !device->status.present) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device [%s] status [%08x]: "
@@ -1070,6 +1088,8 @@ EXPORT_SYMBOL_GPL(acpi_kobj);
 static int __init acpi_init(void)
 {
 	int result;
+	acpi_status status;
+	struct acpi_table_stao *stao_ptr;
 
 	if (acpi_disabled) {
 		printk(KERN_INFO PREFIX "Interpreter disabled.\n");
@@ -1082,6 +1102,22 @@ static int __init acpi_init(void)
 		acpi_kobj = NULL;
 	}
 
+	/* If there is STAO table, check whether it needs to ignore the UART
+	 * device in SPCR table.
+	 */
+	status = acpi_get_table(ACPI_SIG_STAO, 0,
+				(struct acpi_table_header **)&stao_ptr);
+	if (ACPI_SUCCESS(status) && stao_ptr->ignore_uart) {
+		struct acpi_table_spcr *spcr_ptr;
+
+		status = acpi_get_table(ACPI_SIG_SPCR, 0,
+					(struct acpi_table_header **)&spcr_ptr);
+		if (ACPI_SUCCESS(status))
+			spcr_uart_addr = spcr_ptr->serial_port.address;
+		else
+			printk(KERN_WARNING "STAO table present, but SPCR is missing.\n");
+	}
+
 	init_acpi_device_notify();
 	result = acpi_bus_init();
 	if (result) {
-- 
2.0.4

[toc] | [next] | [standalone]

#1328268

On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
>
> ACPI 6.0 introduces a new table STAO to list the devices which are used
> by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> UART is used by Xen. So here it hides UART from Dom0.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Well, this doesn't look right to me.

We need to find a nicer way to achieve what you want.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1328924

On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> >
> > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > UART is used by Xen. So here it hides UART from Dom0.
> >
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> Well, this doesn't look right to me.
> 
> We need to find a nicer way to achieve what you want.

I take that you are talking about how to honor the STAO table in Linux.
Do you have any concrete suggestions?

[toc] | [prev] | [next] | [standalone]

#1329649

On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > >
> > > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > > UART is used by Xen. So here it hides UART from Dom0.
> > >
> > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > Well, this doesn't look right to me.
> > 
> > We need to find a nicer way to achieve what you want.
> 
> I take that you are talking about how to honor the STAO table in Linux.
> Do you have any concrete suggestions?

I do.

The last hunk of the patch is likely what it needs to be, although I'm
not sure if the place it is added to is the right one.  That's a minor thing,
though.

The other part is problematic.  Not that as it doesn't work, but because of
how it works.  With these changes the device will be visible to the OS (in
fact to user space even), but will never be "present".  I'm not sure if
that's what you want?

It might be better to add a check to acpi_bus_type_and_status() that will
evaluate the "should ignore?" thing and return -ENODEV if this is true.  This
way the device won't be visible at all.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[toc] | [prev] | [next] | [standalone]

#1330108

On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
> On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > >
> > > > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > > > UART is used by Xen. So here it hides UART from Dom0.
> > > >
> > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > Well, this doesn't look right to me.
> > > 
> > > We need to find a nicer way to achieve what you want.
> > 
> > I take that you are talking about how to honor the STAO table in Linux.
> > Do you have any concrete suggestions?
> 
> I do.
> 
> The last hunk of the patch is likely what it needs to be, although I'm
> not sure if the place it is added to is the right one.  That's a minor thing,
> though.
> 
> The other part is problematic.  Not that as it doesn't work, but because of
> how it works.  With these changes the device will be visible to the OS (in
> fact to user space even), but will never be "present".  I'm not sure if
> that's what you want?
> 
> It might be better to add a check to acpi_bus_type_and_status() that will
> evaluate the "should ignore?" thing and return -ENODEV if this is true.  This
> way the device won't be visible at all.

Something like below?  Actually your suggestion is better, thank you!

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 78d5f02..4778c51 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
+	if (acpi_check_device_is_ignored(handle))
+		return -ENODEV;
+
 	switch (acpi_type) {
 	case ACPI_TYPE_ANY:		/* for ACPI_ROOT_OBJECT */
 	case ACPI_TYPE_DEVICE:

[toc] | [prev] | [next] | [standalone]

#1331546

On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
> On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
> > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > > >
> > > > > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > > > > UART is used by Xen. So here it hides UART from Dom0.
> > > > >
> > > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > 
> > > > Well, this doesn't look right to me.
> > > > 
> > > > We need to find a nicer way to achieve what you want.
> > > 
> > > I take that you are talking about how to honor the STAO table in Linux.
> > > Do you have any concrete suggestions?
> > 
> > I do.
> > 
> > The last hunk of the patch is likely what it needs to be, although I'm
> > not sure if the place it is added to is the right one.  That's a minor thing,
> > though.
> > 
> > The other part is problematic.  Not that as it doesn't work, but because of
> > how it works.  With these changes the device will be visible to the OS (in
> > fact to user space even), but will never be "present".  I'm not sure if
> > that's what you want?
> > 
> > It might be better to add a check to acpi_bus_type_and_status() that will
> > evaluate the "should ignore?" thing and return -ENODEV if this is true.  This
> > way the device won't be visible at all.
> 
> Something like below?  Actually your suggestion is better, thank you!
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 78d5f02..4778c51 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
>  	if (ACPI_FAILURE(status))
>  		return -ENODEV;
>  
> +	if (acpi_check_device_is_ignored(handle))
> +		return -ENODEV;
> +
>  	switch (acpi_type) {
>  	case ACPI_TYPE_ANY:		/* for ACPI_ROOT_OBJECT */
>  	case ACPI_TYPE_DEVICE:
> 

I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't be
applicable to the other types.  But generally, yes.

Plus I'd move the table checks to acpi_scan_init(), so the UART address can
be a static variable in scan.c.

Also maybe rename acpi_check_device_is_ignored() to something like
acpi_device_should_be_hidden().

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1332200

On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
> On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
> > On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
> > > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> > > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > > > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > >
> > > > > > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > > > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > > > > > UART is used by Xen. So here it hides UART from Dom0.
> > > > > >
> > > > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > > 
> > > > > Well, this doesn't look right to me.
> > > > > 
> > > > > We need to find a nicer way to achieve what you want.
> > > > 
> > > > I take that you are talking about how to honor the STAO table in Linux.
> > > > Do you have any concrete suggestions?
> > > 
> > > I do.
> > > 
> > > The last hunk of the patch is likely what it needs to be, although I'm
> > > not sure if the place it is added to is the right one.  That's a minor thing,
> > > though.
> > > 
> > > The other part is problematic.  Not that as it doesn't work, but because of
> > > how it works.  With these changes the device will be visible to the OS (in
> > > fact to user space even), but will never be "present".  I'm not sure if
> > > that's what you want?
> > > 
> > > It might be better to add a check to acpi_bus_type_and_status() that will
> > > evaluate the "should ignore?" thing and return -ENODEV if this is true.  This
> > > way the device won't be visible at all.
> > 
> > Something like below?  Actually your suggestion is better, thank you!
> > 
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 78d5f02..4778c51 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
> >  	if (ACPI_FAILURE(status))
> >  		return -ENODEV;
> >  
> > +	if (acpi_check_device_is_ignored(handle))
> > +		return -ENODEV;
> > +
> >  	switch (acpi_type) {
> >  	case ACPI_TYPE_ANY:		/* for ACPI_ROOT_OBJECT */
> >  	case ACPI_TYPE_DEVICE:
> > 
> 
> I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't be
> applicable to the other types.  But generally, yes.

I was pondering about it myself. Maybe an ACPI_TYPE_PROCESSOR object
could theoretically be hidden with the STAO? I added the check before
the switch because I thought that there would be no harm in being
caution about it.


> Plus I'd move the table checks to acpi_scan_init(), so the UART address can
> be a static variable in scan.c.
>
> Also maybe rename acpi_check_device_is_ignored() to something like
> acpi_device_should_be_hidden().

Both make sense. Shannon, are you happy to make these changes?

[toc] | [prev] | [next] | [standalone]

#1332407

On Thursday, February 11, 2016 04:04:14 PM Stefano Stabellini wrote:
> On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
> > On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
> > > On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
> > > > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> > > > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > > > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > > > > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > > >
> > > > > > > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > > > > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > > > > > > UART is used by Xen. So here it hides UART from Dom0.
> > > > > > >
> > > > > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > > > 
> > > > > > Well, this doesn't look right to me.
> > > > > > 
> > > > > > We need to find a nicer way to achieve what you want.
> > > > > 
> > > > > I take that you are talking about how to honor the STAO table in Linux.
> > > > > Do you have any concrete suggestions?
> > > > 
> > > > I do.
> > > > 
> > > > The last hunk of the patch is likely what it needs to be, although I'm
> > > > not sure if the place it is added to is the right one.  That's a minor thing,
> > > > though.
> > > > 
> > > > The other part is problematic.  Not that as it doesn't work, but because of
> > > > how it works.  With these changes the device will be visible to the OS (in
> > > > fact to user space even), but will never be "present".  I'm not sure if
> > > > that's what you want?
> > > > 
> > > > It might be better to add a check to acpi_bus_type_and_status() that will
> > > > evaluate the "should ignore?" thing and return -ENODEV if this is true.  This
> > > > way the device won't be visible at all.
> > > 
> > > Something like below?  Actually your suggestion is better, thank you!
> > > 
> > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > index 78d5f02..4778c51 100644
> > > --- a/drivers/acpi/scan.c
> > > +++ b/drivers/acpi/scan.c
> > > @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
> > >  	if (ACPI_FAILURE(status))
> > >  		return -ENODEV;
> > >  
> > > +	if (acpi_check_device_is_ignored(handle))
> > > +		return -ENODEV;
> > > +
> > >  	switch (acpi_type) {
> > >  	case ACPI_TYPE_ANY:		/* for ACPI_ROOT_OBJECT */
> > >  	case ACPI_TYPE_DEVICE:
> > > 
> > 
> > I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't be
> > applicable to the other types.  But generally, yes.
> 
> I was pondering about it myself. Maybe an ACPI_TYPE_PROCESSOR object
> could theoretically be hidden with the STAO?

But this patch won't check for it anyway, will it?

It seems to be only checking against the UART address or have I missed
anything?

> I added the check before
> the switch because I thought that there would be no harm in being
> caution about it.
> 
> 
> > Plus I'd move the table checks to acpi_scan_init(), so the UART address can
> > be a static variable in scan.c.
> >
> > Also maybe rename acpi_check_device_is_ignored() to something like
> > acpi_device_should_be_hidden().
> 
> Both make sense. Shannon, are you happy to make these changes?

Plus maybe make acpi_device_should_be_hidden() print a (KERN_INFO) message
when it decides to hide something?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1332693

On Thu, 11 Feb 2016, Rafael J. Wysocki wrote:
> On Thursday, February 11, 2016 04:04:14 PM Stefano Stabellini wrote:
> > On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
> > > On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
> > > > On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
> > > > > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> > > > > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > > > > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > > > > > > > From: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > > > >
> > > > > > > > ACPI 6.0 introduces a new table STAO to list the devices which are used
> > > > > > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
> > > > > > > > UART is used by Xen. So here it hides UART from Dom0.
> > > > > > > >
> > > > > > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > > > > > > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > > > > 
> > > > > > > Well, this doesn't look right to me.
> > > > > > > 
> > > > > > > We need to find a nicer way to achieve what you want.
> > > > > > 
> > > > > > I take that you are talking about how to honor the STAO table in Linux.
> > > > > > Do you have any concrete suggestions?
> > > > > 
> > > > > I do.
> > > > > 
> > > > > The last hunk of the patch is likely what it needs to be, although I'm
> > > > > not sure if the place it is added to is the right one.  That's a minor thing,
> > > > > though.
> > > > > 
> > > > > The other part is problematic.  Not that as it doesn't work, but because of
> > > > > how it works.  With these changes the device will be visible to the OS (in
> > > > > fact to user space even), but will never be "present".  I'm not sure if
> > > > > that's what you want?
> > > > > 
> > > > > It might be better to add a check to acpi_bus_type_and_status() that will
> > > > > evaluate the "should ignore?" thing and return -ENODEV if this is true.  This
> > > > > way the device won't be visible at all.
> > > > 
> > > > Something like below?  Actually your suggestion is better, thank you!
> > > > 
> > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > > index 78d5f02..4778c51 100644
> > > > --- a/drivers/acpi/scan.c
> > > > +++ b/drivers/acpi/scan.c
> > > > @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
> > > >  	if (ACPI_FAILURE(status))
> > > >  		return -ENODEV;
> > > >  
> > > > +	if (acpi_check_device_is_ignored(handle))
> > > > +		return -ENODEV;
> > > > +
> > > >  	switch (acpi_type) {
> > > >  	case ACPI_TYPE_ANY:		/* for ACPI_ROOT_OBJECT */
> > > >  	case ACPI_TYPE_DEVICE:
> > > > 
> > > 
> > > I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't be
> > > applicable to the other types.  But generally, yes.
> > 
> > I was pondering about it myself. Maybe an ACPI_TYPE_PROCESSOR object
> > could theoretically be hidden with the STAO?
> 
> But this patch won't check for it anyway, will it?
> 
> It seems to be only checking against the UART address or have I missed
> anything?

You are right, this patch only checks for the UART address, which is
critical.

However the STAO also has a "Name List" field with a list of paths in
ACPI namespace to hide. If not implementing proper "Name List" support,
at least, as part of this patch, it would be nice to check for the
presence of the Name List field in the table, and print a warning such
as "STAO Name List not yet supported" when the field is present.

[toc] | [prev] | [next] | [standalone]

#1332706

On Fri, Feb 12, 2016 at 12:50 PM, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Thu, 11 Feb 2016, Rafael J. Wysocki wrote:
>> On Thursday, February 11, 2016 04:04:14 PM Stefano Stabellini wrote:
>> > On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
>> > > On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
>> > > > On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
>> > > > > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
>> > > > > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
>> > > > > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>> > > > > > > > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > > > > > > >
>> > > > > > > > ACPI 6.0 introduces a new table STAO to list the devices which are used
>> > > > > > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the physical
>> > > > > > > > UART is used by Xen. So here it hides UART from Dom0.
>> > > > > > > >
>> > > > > > > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > > > > > > > Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> > > > > > >
>> > > > > > > Well, this doesn't look right to me.
>> > > > > > >
>> > > > > > > We need to find a nicer way to achieve what you want.
>> > > > > >
>> > > > > > I take that you are talking about how to honor the STAO table in Linux.
>> > > > > > Do you have any concrete suggestions?
>> > > > >
>> > > > > I do.
>> > > > >
>> > > > > The last hunk of the patch is likely what it needs to be, although I'm
>> > > > > not sure if the place it is added to is the right one.  That's a minor thing,
>> > > > > though.
>> > > > >
>> > > > > The other part is problematic.  Not that as it doesn't work, but because of
>> > > > > how it works.  With these changes the device will be visible to the OS (in
>> > > > > fact to user space even), but will never be "present".  I'm not sure if
>> > > > > that's what you want?
>> > > > >
>> > > > > It might be better to add a check to acpi_bus_type_and_status() that will
>> > > > > evaluate the "should ignore?" thing and return -ENODEV if this is true.  This
>> > > > > way the device won't be visible at all.
>> > > >
>> > > > Something like below?  Actually your suggestion is better, thank you!
>> > > >
>> > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> > > > index 78d5f02..4778c51 100644
>> > > > --- a/drivers/acpi/scan.c
>> > > > +++ b/drivers/acpi/scan.c
>> > > > @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
>> > > >         if (ACPI_FAILURE(status))
>> > > >                 return -ENODEV;
>> > > >
>> > > > +       if (acpi_check_device_is_ignored(handle))
>> > > > +               return -ENODEV;
>> > > > +
>> > > >         switch (acpi_type) {
>> > > >         case ACPI_TYPE_ANY:             /* for ACPI_ROOT_OBJECT */
>> > > >         case ACPI_TYPE_DEVICE:
>> > > >
>> > >
>> > > I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't be
>> > > applicable to the other types.  But generally, yes.
>> >
>> > I was pondering about it myself. Maybe an ACPI_TYPE_PROCESSOR object
>> > could theoretically be hidden with the STAO?
>>
>> But this patch won't check for it anyway, will it?
>>
>> It seems to be only checking against the UART address or have I missed
>> anything?
>
> You are right, this patch only checks for the UART address, which is
> critical.
>
> However the STAO also has a "Name List" field with a list of paths in
> ACPI namespace to hide. If not implementing proper "Name List" support,
> at least, as part of this patch, it would be nice to check for the
> presence of the Name List field in the table, and print a warning such
> as "STAO Name List not yet supported" when the field is present.

That would be fine, but anyway it belongs to the table parsing part IMO.

And the check can be moved when adding proper support for the name list part.

Thanks,
Rafael

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web