Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1590635 > unrolled thread

Re: [PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for future usage

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for  future usage Thomas Gleixner <tglx@linutronix.de> - 2017-03-01 22:40 +0100
    Re: [PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for  future usage Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-03-01 23:10 +0100

#1590635 — Re: [PATCH linux v3 2/9] x86/acpi: store ACPI ids from MADT for future usage

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

[toc] | [next] | [standalone]

#1590649

On 03/01/2017 02:54 PM, Thomas Gleixner wrote:
>
> Yes, I know it has been fixed later, but this crap should not have been
> merged in the first place.
>
> Yours grumpy
>
>       tglx


Yes, we dropped the ball on this, this shouldn't have gone in without
x86 maintainers' review.

-boris

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web