Thread (34 messages) 34 messages, 5 authors, 2016-02-29

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

From: Stefano Stabellini <hidden>
Date: 2016-02-11 16:04:44
Also in: linux-acpi, linux-devicetree, linux-efi, lkml

On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
quoted
On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
quoted
On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
quoted
On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
quoted
On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao [off-list ref] wrote:
quoted
From: Shannon Zhao <redacted>

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 <redacted>
Reviewed-by: Stefano Stabellini <redacted>
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?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help