Message-ID: <20110201110026.GA4739@liondog.tnic>
Date:	Tue, 1 Feb 2011 12:00:26 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Jeremy Fitzhardinge <jeremy@...p.org>,
	"H. Peter Anvin" <hpa@...or.com>
Cc:	Ingo Molnar <mingo@...e.hu>,
	the arch/x86 maintainers <x86@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Xen Devel <Xen-devel@...ts.xensource.com>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>
Subject: Re: [PATCH 0/2] x86/microcode: support for microcode update in Xen
 dom0

On Mon, Jan 31, 2011 at 04:15:21PM -0800, Jeremy Fitzhardinge wrote:
> My understanding of what you're proposing is:
> 
>    1. the normal CPU-specific microcode driver starts up
>    2. an if (xen) test overrides the apply_microcode to call a
>       Xen-specific function
>    3. the driver requests the firmware, loads and verifies it as normal
>    4. the microcode is applied via the Xen apply_microcode function
> 
> With (2) needing a Xen-specific change to both the Intel and AMD drivers.
> 
> If not, what are you proposing in detail?
> 
> Are you instead thinking:
> 
>     * Xen-specific microcode driver starts up
>     * It calls either the intel or amd request_microcode_fw as needed
>     * apply the the loaded data via hypercall
> 
> ?

I am thinking something in the sense of the above. For example, in the
AMD case you take

static struct microcode_ops microcode_amd_ops = {
	.request_microcode_user           = request_microcode_user,
	.request_microcode_fw             = request_microcode_fw,
	.collect_cpu_info                 = collect_cpu_info_amd,
	.apply_microcode                  = apply_microcode_amd,
	.microcode_fini_cpu               = microcode_fini_cpu_amd,
};

and reuse the ->request_microcode_fw, ->collect_cpu_info and
->microcode_fini_cpu on dom0 as if you're running on baremetal. Up
to the point where you need to apply the microcode. Then, you use
your supplied ->apply_microcode hypercall wrapper to call into the
hypervisor.

> But all this is flawed because the microcode_intel/amd.c drivers assume
> they can see all the CPUs present in the system, and load suitable
> microcode for each specific one.  But a kernel in a Xen domain only has
> virtual CPUs - not physical ones - and has no idea how to get
> appropriate microcode data for all the physical CPUs in the system.

Well, let me quote you:

On Fri, Jan 28, 2011 at 04:26:52PM -0800, Jeremy Fitzhardinge wrote:
> Xen update mechanism is uniform independent of the CPU type, but the
> driver must know where to find the data file, which depends on the CPU
> type. And since the update hypercall updates all CPUs, we only need to
> execute it once on any CPU - but for simplicity it just runs it only
> on (V)CPU 0.

so you only do it once and exit early in the rest of the cases. I
wouldn't worry about performance since ucode is applied only once upon
boot.

[..]

> Right.  There's nothing really for the driver to do (kernel or Xen). 
> The Intel driver does a bunch of checksum checking, which is presumably
> useful for detecting a corrupted firmware update early, and splits out
> the chunks specific to the current cpus.  The AMD driver does no
> checking per-se, but also looks for processor-specific microcode update
> chunks.
> 
> In the Xen case, since a domain is not necessarily seeing all the
> details of the underlying physical cpus, it makes most sense for
> usermode to just pass in the whole microcode file and let Xen do all the
> checking/filtering based on complete knowledge of the system.

This is exactly what I'm talking about - why copy all that
checking/filtering code from baremetal to Xen instead of simply reusing
it? Especially if you'd need to update the copy from time to time when
baremetal changes.

> > All I'm saying is, you should try as best as possible to avoid code
> > duplication and the need for replicating functionality to Xen, thus
> > doubling - even multiplying - the effort for coding/testing baremetal
> > and then Xen. Microcode is a perfect example since the vendors do all
> > their testing/verification on baremetal anyway and the rest should
> > benefit from that work.
> 
> CPU vendors test Xen, and Intel is particularly interested in getting
> this microcode driver upstream.  The amount of duplicated code is
> trivial, and the basic structure of the microcode updates doesn't seem
> set to change.

Uuh, I wouldn't bet on that though :).

> Since Xen has to have all sorts of other CPU-specific code which at
> least somewhat overlaps with what's in the kernel a bit more doesn't
> matter.

Well, I'll let x86 people decide on that but last time I checked they
opposed "if (xen)" sprinkling all over the place.

Btw, hpa has a point, if you can load microcode using multiboot, all
that discussion will become moot since you'll be better at loading
microcode even than baremetal. We need a similar mechanism in x86 too
since the current one loads the microcode definitely too late.

The optimal case for baremetal would be to load it as early as possible
on each CPU's bootstrapping path and if you can do that in the
hypervisor, before even dom0 starts, you're very much fine.

> (It's interesting that nobody seems to have been interested enough in
> Via to have ever implemented a microcode driver for it - perhaps they
> only ever do it from BIOS.)

Yeah, that's one possibility. I bet they'll do it though, when BIOS
cannot be updated/is out of life on some of their platforms and they
want to get a ucode patch applied.

-- 
Regards/Gruss,
    Boris.
--
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/

Powered by blists - more mailing lists