linux-kernel - Re: [PATCH V8 5/6] ACPI: Support the probing on the devices which apply indirect-IO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a73c87d3-59fc-843b-a4e1-d9f5201bffd9@gmail.com>
Date:   Fri, 21 Apr 2017 10:22:52 +0800
From:   "zhichang.yuan" <zhichang.yuan02@...il.com>
To:     dann frazier <dann.frazier@...onical.com>,
        "zhichang.yuan" <yuanzhichang@...ilicon.com>,
        lorenzo.pieralisi@....com, hanjun.guo@...aro.org,
        sudeep.holla@....com
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>, rafael@...nel.org,
        Arnd Bergmann <arnd@...db.de>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Mark Rutland <mark.rutland@....com>,
        Brian Starkey <brian.starkey@....com>, olof@...om.net,
        benh@...nel.crashing.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-acpi@...r.kernel.org, linuxarm@...wei.com,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        linux-pci@...r.kernel.org, Corey Minyard <minyard@....org>,
        Zou Rongrong <zourongrong@...il.com>,
        John Garry <john.garry@...wei.com>,
        Gabriele Paoloni <gabriele.paoloni@...wei.com>,
        kantyzc@....com, xuwei5@...ilicon.com,
        Seth Forshee <seth.forshee@...onical.com>
Subject: Re: [PATCH V8 5/6] ACPI: Support the probing on the devices which
 apply indirect-IO

Hi, Dann,



On 04/21/2017 04:57 AM, dann frazier wrote:
> On Thu, Mar 30, 2017 at 9:26 AM, zhichang.yuan
> <yuanzhichang@...ilicon.com> wrote:
>> On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access I/O
>> with some special host-local I/O ports known on x86. To access the I/O
>> peripherals, an indirect-IO mechanism is introduced to mapped the host-local
>> I/O to system logical/fake PIO similar the PCI MMIO on architectures where no
>> separate I/O space exists. Just as PCI MMIO, the host I/O range should be
>> registered before probing the downstream devices and set up the I/O mapping.
>> But current ACPI bus probing doesn't support these indirect-IO hosts/devices.
>>
>> This patch introdueces a new ACPI handler for this device category. Through the
>> handler attach callback, the indirect-IO hosts I/O registration is done and
>> all peripherals' I/O resources are translated into logic/fake PIO before
>> starting the enumeration.
>>
>> Signed-off-by: zhichang.yuan <yuanzhichang@...ilicon.com>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@...wei.com>
>> ---
>>  drivers/acpi/Makefile          |   1 +
>>  drivers/acpi/acpi_indirectio.c | 344 +++++++++++++++++++++++++++++++++++++++++
>>  drivers/acpi/internal.h        |   5 +
>>  drivers/acpi/scan.c            |   1 +
>>  4 files changed, 351 insertions(+)
>>  create mode 100644 drivers/acpi/acpi_indirectio.c
>>
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index a391bbc..10e5f2b 100644
>> --- a/drivers/acpi/Makefile
>> +++ b/drivers/acpi/Makefile
>> @@ -57,6 +57,7 @@ acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
>>  acpi-y                         += acpi_lpat.o
>>  acpi-$(CONFIG_ACPI_GENERIC_GSI) += irq.o
>>  acpi-$(CONFIG_ACPI_WATCHDOG)   += acpi_watchdog.o
>> +acpi-$(CONFIG_INDIRECT_PIO)    += acpi_indirectio.o
>>
>>  # These are (potentially) separate modules
>>
>> diff --git a/drivers/acpi/acpi_indirectio.c b/drivers/acpi/acpi_indirectio.c
>> new file mode 100644
>> index 0000000..c8c80b5
>> --- /dev/null
>> +++ b/drivers/acpi/acpi_indirectio.c
>> @@ -0,0 +1,344 @@

[snip]

>> +acpi_build_logiciores_template(struct acpi_device *adev,
>> +                       struct acpi_buffer *buffer)
>> +{
>> +       acpi_handle handle = adev->handle;
>> +       struct acpi_resource *resource;
>> +       acpi_status status;
>> +       int res_cnt = 0;
>> +
>> +       status = acpi_walk_resources(handle, METHOD_NAME__CRS,
>> +                                    acpi_count_logiciores, &res_cnt);
>> +       if (ACPI_FAILURE(status) || !res_cnt) {
>> +               dev_err(&adev->dev, "can't evaluate _CRS: %d\n", status);
>> +               return -EINVAL;
>> +       }
>> +
>> +       buffer->length = sizeof(struct acpi_resource) * (res_cnt + 1) + 1;
>> +       buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);
> 
> (Seth Forshee noticed this issue, just passing it on)
> 
> Should this just allocate the full buffer->length? That would keep the
> length attribute accurate (possibly avoiding an off-by-1 error later).
> It's not clear what the trailing byte is needed for, but other drivers
> allocate it as well (drivers/acpi/pci_link.c and
> drivers/platform/x86/sony-laptop.c).

Thanks for your suggestion!

I also curious why this one appended byte is needed as it seems the later
acpi_set_current_resources() doesn't use this byte.
And I tested without setting the buffer->length as the length of resource list
directly, it seems ok.

But anyway, it looks more reasonable to allocate the memory with the
buffer->length rather than buffer->length - 1;

I was made the V9 patch-set, and I can add your suggestion there. But I also
awaiting for ARM64 ACPI maintainer's comment about this patch before really
sending V9. I wonder whether there is better way to make our indirect-IO devices
can be assigned the logic PIO before the enumeration...

Lorenzo, Hanjun, what do you think about this patch?

Thanks,
Zhichang

> 
>  -dann
>