linux-kernel - Re: [PATCH v3 5/7] xen: add capability to remap non-RAM pages to different PFNs

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: <7ce2eaaf-27a7-47a6-bf33-87176cd9cb26@suse.com>
Date: Tue, 10 Sep 2024 14:51:20 +0200
From: Jürgen Groß <jgross@...e.com>
To: Jan Beulich <jbeulich@...e.com>
Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>,
 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>, xen-devel@...ts.xenproject.org,
 linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [PATCH v3 5/7] xen: add capability to remap non-RAM pages to
 different PFNs

On 10.09.24 14:26, Jan Beulich wrote:
> On 10.09.2024 12:39, Juergen Gross wrote:
>> When running as a Xen PV dom0 it can happen that the kernel is being
>> loaded to a guest physical address conflicting with the host memory
>> map.
>>
>> In order to be able to resolve this conflict, add the capability to
>> remap non-RAM areas to different guest PFNs. A function to use this
>> remapping information for other purposes than doing the remap will be
>> added when needed.
>>
>> As the number of conflicts should be rather low (currently only
>> machines with max. 1 conflict are known), save the remap data in a
>> small statically allocated array.
>>
>> Signed-off-by: Juergen Gross <jgross@...e.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@...e.com>
> with two cosmetic remarks:
> 
>> @@ -792,6 +793,70 @@ int clear_foreign_p2m_mapping(struct gnttab_unmap_grant_ref *unmap_ops,
>>   	return ret;
>>   }
>>   
>> +/* Remapped non-RAM areas */
>> +#define NR_NONRAM_REMAP 4
>> +static struct nonram_remap {
>> +	phys_addr_t maddr;
>> +	phys_addr_t paddr;
>> +	size_t size;
>> +} xen_nonram_remap[NR_NONRAM_REMAP] __ro_after_init;
>> +static unsigned int nr_nonram_remap __ro_after_init;
> 
> I take it this is the canonical placement of section attributes in the
> kernel? (In Xen I'd ask for the attributes to be moved ahead of the
> identifiers being declared.)

I didn't find an explicit mentioning in the coding style, but most
examples I've found place the section attribute after the name of the
variable.

> 
>> +/*
>> + * Do the real remapping of non-RAM regions as specified in the
>> + * xen_nonram_remap[] array.
>> + * In case of an error just crash the system.
>> + */
>> +void __init xen_do_remap_nonram(void)
>> +{
>> +	unsigned int i;
>> +	unsigned int remapped = 0;
>> +	const struct nonram_remap *remap = xen_nonram_remap;
>> +	unsigned long pfn, mfn, end_pfn;
>> +
>> +	for (i = 0; i < nr_nonram_remap; i++) {
>> +		end_pfn = PFN_UP(remap->paddr + remap->size);
>> +		pfn = PFN_DOWN(remap->paddr);
>> +		mfn = PFN_DOWN(remap->maddr);
>> +		while (pfn < end_pfn) {
>> +			if (!set_phys_to_machine(pfn, mfn)) {
>> +				pr_err("Failed to set p2m mapping for pfn=%lx mfn=%lx\n",
>> +				       pfn, mfn);
>> +				BUG();
> 
> Wouldn't panic() get you both in one operation? Or do you expect the call
> trace / register state to be of immediate relevance for analysis?

You are right, in this case panic() is a better option.


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)