linux-kernel - Re: [PATCH v3 6/7] xen: allow mapping ACPI data using a different physical address

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8ca5189f-6e6b-45fb-938d-0f5e21e8bb72@suse.com>
Date: Tue, 10 Sep 2024 14:52:56 +0200
From: Jürgen Groß <jgross@...e.com>
To: Jan Beulich <jbeulich@...e.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H. Peter Anvin" <hpa@...or.com>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Len Brown <lenb@...nel.org>, Boris Ostrovsky <boris.ostrovsky@...cle.com>,
 xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
 x86@...nel.org, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v3 6/7] xen: allow mapping ACPI data using a different
 physical address

On 10.09.24 14:34, Jan Beulich wrote:
> On 10.09.2024 12:39, Juergen Gross wrote:
>> When running as a Xen PV dom0 the system needs to map ACPI data of the
>> host using host physical addresses, while those addresses can conflict
>> with the guest physical addresses of the loaded linux kernel. The same
>> problem might apply in case a PV guest is configured to use the host
>> memory map.
>>
>> This conflict can be solved by mapping the ACPI data to a different
>> guest physical address, but mapping the data via acpi_os_ioremap()
>> must still be possible using the host physical address, as this
>> address might be generated by AML when referencing some of the ACPI
>> data.
>>
>> When configured to support running as a Xen PV domain, have an
>> implementation of acpi_os_ioremap() being aware of the possibility to
>> need above mentioned translation of a host physical address to the
>> guest physical address.
>>
>> This modification requires to fix some #include of asm/acpi.h in x86
>> code to use linux/acpi.h instead.
>>
>> Signed-off-by: Juergen Gross <jgross@...e.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@...e.com>
> with a request to comment a tiny bit more:
> 
>> @@ -836,6 +837,33 @@ void __init xen_do_remap_nonram(void)
>>   	pr_info("Remapped %u non-RAM page(s)\n", remapped);
>>   }
>>   
>> +#ifdef CONFIG_ACPI
>> +/*
>> + * Xen variant of acpi_os_ioremap() taking potentially remapped non-RAM
>> + * regions into acount.
> 
> (Nit: account)

Indeed.

> 
>> + * Any attempt to map an area crossing a remap boundary will produce a
>> + * WARN() splat.
>> + */
>> +static void __iomem *xen_acpi_os_ioremap(acpi_physical_address phys,
>> +					 acpi_size size)
>> +{
>> +	unsigned int i;
>> +	const struct nonram_remap *remap = xen_nonram_remap;
>> +
>> +	for (i = 0; i < nr_nonram_remap; i++) {
>> +		if (phys + size > remap->maddr &&
>> +		    phys < remap->maddr + remap->size) {
>> +			WARN_ON(phys < remap->maddr ||
>> +				phys + size > remap->maddr + remap->size);
>> +			phys = remap->paddr + phys - remap->maddr;
> 
> This might be slightly easier / more logical to read as
> 
> 			phys += remap->paddr - remap->maddr;
> 
> Also because of "phys" not consistently expressing a physical address
> (when you need convert it, the incoming value is a machine address) a
> comment may help here. In fact at the first glance (and despite having
> seen the code before) I thought the translation was done the wrong way
> round, simply because of the name of the variable.

Will add a comment and change the line as you suggest.


Juergen

Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3684 bytes)

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (496 bytes)