linux-kernel - Re: [Resend PATCH 2/5] ACPI/Resource: Add address translation support

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: <20131016230515.GB17866@google.com>
Date:	Wed, 16 Oct 2013 17:05:15 -0600
From:	Bjorn Helgaas <bhelgaas@...gle.com>
To:	tianyu.lan@...el.com
Cc:	lenb@...nel.org, rjw@...k.pl, yinghai@...nel.org,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [Resend PATCH 2/5] ACPI/Resource: Add address translation support

On Fri, Oct 11, 2013 at 08:18:58PM +0800, tianyu.lan@...el.com wrote:
> From: Lan Tianyu <tianyu.lan@...el.com>
> 
> According ACPI 5.0 spec Section 19.1.8

This section reference is wrong.  Table 5-133 (on page 243) does
point to Section 19.1.8, but that section is only the ASL grammar
description and doesn't have any useful information about what
_TRA means.

A more useful reference (and the source of the quote below) is
Section 6.4.3.5.1.

> "For bridges, translate addresses across the bridge, this is the
> offset that must be added to the address on the secondary side
> to obtain the address on the primary side. Non-bridge devices
> must list 0."

You didn't quote this correctly.  Here's a copy/paste from the spec:

    For bridges that translate addresses across the bridge, this is
    the offset that must be added to the address on the secondary side
    to obtain the address on the primary side. Non-bridge devices must
    list 0 for all Address Translation offset bits.

> This patch is to add address translation offset to the start/end
> of struct resource in the acpi_dev_resource_address_space().
> Further more, non-bridge device's translation_offset should 0.
> So this change will affect other devices.
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@...el.com>

I like the patch, though :)  With the above corrections,

Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>

> ---
>  drivers/acpi/resource.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 929f416..84bc3db 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -192,8 +192,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>  	if (ACPI_FAILURE(status))
>  		return true;
>  
> -	res->start = addr.minimum;
> -	res->end = addr.maximum;
> +	res->start = addr.minimum + addr.translation_offset;
> +	res->end = addr.maximum + addr.translation_offset;
>  	window = addr.producer_consumer == ACPI_PRODUCER;
>  
>  	switch(addr.resource_type) {
> -- 
> 1.8.2.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/