Re: [Qemu-devel] [PATCH 5/6] [wip] tseg, part1, not (yet) tested

[Laszlo Ersek]

On 04/21/15 17:04, Gerd Hoffmann wrote:
>>> +static const MemoryRegionOps tseg_blackhole_ops = {
>>> +    .read = tseg_blackhole_read,
>>> +    .write = tseg_blackhole_write,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>> +    .valid.min_access_size = 1,
>>> +    .valid.max_access_size = 4,
>>> +    .impl.min_access_size = 4,
>>> +    .impl.max_access_size = 4,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +};
>>
>> (a) you've specified .endianness twice (with inconsistent initializers
>> at that, although the value returned by the accessor happens to be
>> unaffected).
> 
> Oops, cut+paste bug, will fix (/me wonders why gcc didn't throw an error
> on that one).

I think because it's valid C99. The "current object" concept is just
moved by the designation, and you can initialize the "current object".

>> - can the guest somehow use this facility to detect, dynamically, the
>> presence of this feature? (For now I'm thinking about a static build
>> flag for OVMF that would enable SMM support, but I'm 99% sure Jordan
>> will object and ask for a dynamic feature detection.)
> 
> Hmm.  I think if we merge all the smm / smram / tseg patches in one go
> this should work.  Without patches reading the ESMRAMC register returns
> zero.  With the patches the three cache-disable bits are hardcoded to
> '1'.  This should work for detecting tseg support.
> 
> You also have to test for smm support.  The current protocol for this is
> that seabios checks whenever smm is already initialized (see
> *_apmc_smm_setup() in seabios/src/fw/smm.c) and if so it skips smm
> initialization.  Right now qemu sets the bit on reset when running on
> kvm, so seabios doesn't try to use smm.  On tcg the bit is clear after
> reset and seabios actually uses smm mode.

Thanks. I'll stash the rest of this thread for later.

A "unified" patchset (not necessarily posted, just pushed somewhere)
would be helpful down the road -- IIRC this one was claimed unapplicable
to current master. (Which is also why I didn't try the "info mtree"
command, see elsewhere in the thread -- I didn't try to build the series.)

Thanks!
Laszlo