linux-kernel - Re: [RFC PATCH v2 06/32] x86/pci: Use memremap when walking setup data

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]

Date:   Fri, 3 Mar 2017 15:15:34 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Bjorn Helgaas <helgaas@...nel.org>,
        Brijesh Singh <brijesh.singh@....com>
CC:     <simon.guinot@...uanux.org>, <linux-efi@...r.kernel.org>,
        <kvm@...r.kernel.org>, <rkrcmar@...hat.com>,
        <matt@...eblueprint.co.uk>, <linux-pci@...r.kernel.org>,
        <linus.walleij@...aro.org>, <gary.hook@....com>,
        <linux-mm@...ck.org>, <paul.gortmaker@...driver.com>,
        <hpa@...or.com>, <cl@...ux.com>, <dan.j.williams@...el.com>,
        <aarcange@...hat.com>, <sfr@...b.auug.org.au>,
        <andriy.shevchenko@...ux.intel.com>, <herbert@...dor.apana.org.au>,
        <bhe@...hat.com>, <xemul@...allels.com>, <joro@...tes.org>,
        <x86@...nel.org>, <peterz@...radead.org>, <piotr.luc@...el.com>,
        <mingo@...hat.com>, <msalter@...hat.com>,
        <ross.zwisler@...ux.intel.com>, <bp@...e.de>, <dyoung@...hat.com>,
        <jroedel@...e.de>, <keescook@...omium.org>, <arnd@...db.de>,
        <toshi.kani@....com>, <mathieu.desnoyers@...icios.com>,
        <luto@...nel.org>, <devel@...uxdriverproject.org>,
        <bhelgaas@...gle.com>, <tglx@...utronix.de>, <mchehab@...nel.org>,
        <iamjoonsoo.kim@....com>, <labbott@...oraproject.org>,
        <tony.luck@...el.com>, <alexandre.bounine@....com>,
        <kuleshovmail@...il.com>, <linux-kernel@...r.kernel.org>,
        <mcgrof@...nel.org>, <mst@...hat.com>,
        <linux-crypto@...r.kernel.org>, <tj@...nel.org>,
        <pbonzini@...hat.com>, <akpm@...ux-foundation.org>,
        <davem@...emloft.net>
Subject: Re: [RFC PATCH v2 06/32] x86/pci: Use memremap when walking setup
 data

On 3/3/2017 2:42 PM, Bjorn Helgaas wrote:
> On Thu, Mar 02, 2017 at 10:13:10AM -0500, Brijesh Singh wrote:
>> From: Tom Lendacky <thomas.lendacky@....com>
>>
>> The use of ioremap will force the setup data to be mapped decrypted even
>> though setup data is encrypted.  Switch to using memremap which will be
>> able to perform the proper mapping.
>
> How should callers decide whether to use ioremap() or memremap()?
>
> memremap() existed before SME and SEV, and this code is used even if
> SME and SEV aren't supported, so the rationale for this change should
> not need the decryption argument.

When SME or SEV is active an ioremap() will remove the encryption bit
from the pagetable entry when it is mapped.  This allows MMIO, which
doesn't support SME/SEV, to be performed successfully.  So my take is
that ioremap() should be used for MMIO and memremap() for pages in RAM.

>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>> ---
>>  arch/x86/pci/common.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
>> index a4fdfa7..0b06670 100644
>> --- a/arch/x86/pci/common.c
>> +++ b/arch/x86/pci/common.c
>> @@ -691,7 +691,7 @@ int pcibios_add_device(struct pci_dev *dev)
>>
>>  	pa_data = boot_params.hdr.setup_data;
>>  	while (pa_data) {
>> -		data = ioremap(pa_data, sizeof(*rom));
>> +		data = memremap(pa_data, sizeof(*rom), MEMREMAP_WB);
>
> I can't quite connect the dots here.  ioremap() on x86 would do
> ioremap_nocache().  memremap(MEMREMAP_WB) would do arch_memremap_wb(),
> which is ioremap_cache().  Is making a cacheable mapping the important
> difference?

The memremap(MEMREMAP_WB) will actually check to see if it can perform
a __va(pa_data) in try_ram_remap() and then fallback to the
arch_memremap_wb().  So it's actually the __va() vs the ioremap_cache()
that is the difference.

Thanks,
Tom

>
>>  		if (!data)
>>  			return -ENOMEM;
>>
>> @@ -710,7 +710,7 @@ int pcibios_add_device(struct pci_dev *dev)
>>  			}
>>  		}
>>  		pa_data = data->next;
>> -		iounmap(data);
>> +		memunmap(data);
>>  	}
>>  	set_dma_domain_ops(dev);
>>  	set_dev_domain_options(dev);
>>