linux-kernel - Re: [Xen-devel] [PATCH 2/2] xen: before ballooning hotplugged memory, set frames to invalid

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: <550B09F7.6060209@citrix.com>
Date:	Thu, 19 Mar 2015 17:40:07 +0000
From:	David Vrabel <david.vrabel@...rix.com>
To:	Juergen Gross <jgross@...e.com>,
	David Vrabel <david.vrabel@...rix.com>,
	<linux-kernel@...r.kernel.org>, <xen-devel@...ts.xensource.com>,
	<konrad.wilk@...cle.com>, <boris.ostrovsky@...cle.com>,
	<daniel.kiper@...cle.com>
Subject: Re: [Xen-devel] [PATCH 2/2] xen: before ballooning hotplugged memory,
 set frames to invalid

On 19/03/15 17:19, Juergen Gross wrote:
> On 03/19/2015 05:18 PM, David Vrabel wrote:
>> On 19/03/15 14:31, Juergen Gross wrote:
>>> Commit 25b884a83d487fd62c3de7ac1ab5549979188482 ("x86/xen: set
>>> regions above the end of RAM as 1:1") introduced a regression.
>>>
>>> To be able to add memory pages which were added via memory hotplug to
>>> a pv domain, the pages must be "invalid" instead of "identity" in the
>>> p2m list before they can be added.
>> [...]
>>> --- a/drivers/xen/balloon.c
>>> +++ b/drivers/xen/balloon.c
>>> @@ -221,15 +221,24 @@ static bool balloon_is_inflated(void)
>>>
>>>   static enum bp_state reserve_additional_memory(long credit)
>>>   {
>>> -    int nid, rc;
>>> +    int nid, rc = 0;
>>>       u64 hotplug_start_paddr;
>>>       unsigned long balloon_hotplug = credit;
>>> +    unsigned long pfn;
>>>
>>>       hotplug_start_paddr = PFN_PHYS(SECTION_ALIGN_UP(max_pfn));
>>>       balloon_hotplug = round_up(balloon_hotplug, PAGES_PER_SECTION);
>>>       nid = memory_add_physaddr_to_nid(hotplug_start_paddr);
>>>
>>> -    rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug <<
>>> PAGE_SHIFT);
>>> +    for (pfn = PFN_DOWN(hotplug_start_paddr);
>>> +         !rc && pfn < PFN_DOWN(hotplug_start_paddr) + balloon_hotplug;
>>> +         pfn++)
>>> +        if (!set_phys_to_machine(pfn, INVALID_P2M_ENTRY))
>>> +            rc = 1;
>>
>>      rc = -ENOMEM;
> 
> I used the value 1 on purpose to be able to identify the case by the
> value printed in the warning below.

Ok.

> 
>>      break;
> 
> Why? !rc is already tested in the for() clause.

I prefer an explicit break.

>>> +
>>> +    if (!rc)
>>> +        rc = add_memory(nid, hotplug_start_paddr,
>>> +                balloon_hotplug << PAGE_SHIFT);
>>
>> Use else here.
> 
> Huh? I want the message to be printed if either set_phys_to_machine()
> or add_memory() failed.

Ok.

David
--
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/