Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753432AbdCAVfY (ORCPT ); Wed, 1 Mar 2017 16:35:24 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:41437 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753313AbdCAVeq (ORCPT ); Wed, 1 Mar 2017 16:34:46 -0500 Date: Wed, 1 Mar 2017 20:54:44 +0100 (CET) From: Thomas Gleixner To: Vitaly Kuznetsov cc: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, x86@kernel.org, Stefano Stabellini , Boris Ostrovsky , David Vrabel , Juergen Gross , Ingo Molnar , "H. Peter Anvin" , Jan Beulich , Andrew Cooper , Julien Grall Subject: Re: [PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for future usage In-Reply-To: <1469536228-29932-3-git-send-email-vkuznets@redhat.com> Message-ID: References: <1469536228-29932-1-git-send-email-vkuznets@redhat.com> <1469536228-29932-3-git-send-email-vkuznets@redhat.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1768 Lines: 51 On Tue, 26 Jul 2016, Vitaly Kuznetsov wrote: So this patch made it's way into Linus tree via XEN w/o an ack or reviewed by from the x86 maintainers. Yes, we were on CC, but it's not that hard to ping the maintainers when they do not respond on a particular patch. The whole series ran under the cover letter subject: xen: pvhvm: support bootup on secondary vCPU which suggests that this is a XEN internal affair. And I really have enough stuff to look after so I don't dive into XEN internals if it's not obviously required. Let's look at this after the fact: > Currently we don't save ACPI ids (unlike LAPIC ids which go to > x86_cpu_to_apicid) from MADT and we may need this information later. may need? Maybe, or maybe not. > Particularly, ACPI ids is the only existent way for a PVHVM Xen guest > to figure out Xen's idea of its vCPUs ids before these CPUs boot and > in some cases these ids diverge from Linux's cpu ids. I have no idea what this sentence means and what kind of divergence this is talking about. Dammit, if stuff gets slammed into the x86 tree w/o a proper notice, then the minimum requirement is at least an understandable changelog which allows non XEN experts to figure out WHY this is necessary and WHAT this is about. > @@ -714,7 +722,7 @@ int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, int *pcpu) > { > int cpu; > > - cpu = acpi_register_lapic(physid, ACPI_MADT_ENABLED); > + cpu = acpi_register_lapic(physid, U32_MAX, ACPI_MADT_ENABLED); What the heck is this? ACPIID is U32_MAX? Sure, that's obvious as it can get and the well thought out comment above this call explains it nicely. Yes, I know it has been fixed later, but this crap should not have been merged in the first place. Yours grumpy tglx