From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752352AbcBLLuy (ORCPT ); Fri, 12 Feb 2016 06:50:54 -0500 Received: from smtp.citrix.com ([66.165.176.89]:51426 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751746AbcBLLuv (ORCPT ); Fri, 12 Feb 2016 06:50:51 -0500 X-IronPort-AV: E=Sophos;i="5.22,435,1449532800"; d="scan'208";a="331328257" Date: Fri, 12 Feb 2016 11:50:24 +0000 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: "Rafael J. Wysocki" CC: Stefano Stabellini , "Rafael J. Wysocki" , Shannon Zhao , "linux-arm-kernel@lists.infradead.org" , , , , Catalin Marinas , Will Deacon , Mark Rutland , , , "devicetree@vger.kernel.org" , , Linux Kernel Mailing List , , Shannon Zhao , , Len Brown , "open list:ACPI" Subject: Re: [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen In-Reply-To: <1667812.hY9vsC2epc@vostro.rjw.lan> Message-ID: References: <1454641552-12576-1-git-send-email-zhaoshenglong@huawei.com> <44113385.2QnPtvxQx0@vostro.rjw.lan> <1667812.hY9vsC2epc@vostro.rjw.lan> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-DLP: MIA1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 wrote: > > > > > > > > From: Shannon Zhao > > > > > > > > > > > > > > > > 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 > > > > > > > > Reviewed-by: Stefano Stabellini > > > > > > > > > > > > > > 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.