linux-kernel - RE: [PATCH V2 2/2] Xen ACPI memory hotplug

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] [day] [month] [year] [list]

Message-ID: <DE8DF0795D48FD4CA783C40EC82923353C2472@SHSMSX101.ccr.corp.intel.com>
Date:	Mon, 14 Jan 2013 06:00:50 +0000
From:	"Liu, Jinsong" <jinsong.liu@...el.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Konrad Rzeszutek Wilk <konrad@...nel.org>
Subject: RE: [PATCH V2 2/2] Xen ACPI memory hotplug

Agree, w/ minor comments below.


Konrad Rzeszutek Wilk wrote:
>> +
>> +	/*
>> +	 * Early boot code has recognized memory area by EFI/E820.
>> +	 * If DSDT shows these memory devices on boot, hotplug is not
>> necessary +	 * for them. So, it just returns until completion of
>> this driver's +	 * start up.
> 
> "So it just returns until completion of this drivers's start up."
> 
> Can you change that to be:
> "Return OK until this driver starts up."
> 
> But then.. how can this function be called with
> acpi_hotmem_initialized=false? Is it b/c of the acpi_walk_namespace
> call? How about you state that: 
> 
> "This can be done via the acpi_walk_namespace which is called during
> early boot and acpi_hotmem_initialized is set _after_ that call
> has completed."
> 

The action for booting existed memory and hot-added memory is different.
For booting existed memory devices, we don't need do xen_hotadd_memory.
For hot-added memory devices, we need do xen_hotadd_memory, hypercall to hypervisor to add memory.

I've updated comments as below to be more clear:
        /*
         * For booting existed memory devices, early boot code has recognized
         * memory area by EFI/E820. If DSDT shows these memory devices on boot,
         * hotplug is not necessary for them.
         * For hot-added memory devices during runtime, it need hypercall to
         * Xen hypervisor to add memory.
         */


>> +static void __exit xen_acpi_memory_device_exit(void) +{
>> +	acpi_status status;
>> +
>> +	if (!xen_initial_domain())
>> +		return;
>> +
>> +	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
>> +				     ACPI_UINT32_MAX, +				    
>> acpi_memory_deregister_notify_handler, +				     NULL, NULL, NULL);
>> +	if (ACPI_FAILURE(status))
>> +		pr_warn(PREFIX "walk_namespace failed\n");
>> +
>> +	acpi_bus_unregister_driver(&xen_acpi_memory_device_driver); +
>> +	/*
>> +	 * stub reserve space again to prevent any chance of native
>> +	 * driver loading, though not much meaning in real life
> 
> not much meaning in real life? What does that mean?
> 

Hmm, what I want to say here is, in real life module driver seldomly unloaded.
Anyway, the comments is ambiguous, I've dropped the word 'though not much meaning in real life'.

Thanks,
Jinsong

>> +	 */
>> +	acpi_bus_register_driver(&xen_stub_memory_device_driver); +	return;
>> +}
>> +
--
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/