Re: [Xen-devel] [PATCH] VTd/dmar: Tweak how the DMAR table is clobbered

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/04/15 08:50, Jan Beulich wrote:
>>>> On 10.04.15 at 11:08, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 10/04/15 02:23, Tian, Kevin wrote:
>>>> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
>>>> Sent: Thursday, April 09, 2015 3:45 AM
>>>>
>>>> Intead of clobbering DMAR -> XMAR and back, clobber to RMAD instead.
>>>> This
>>>> means that changing the signature does not alter the checksum, which allows
>>>> the clobbering/unclobbering to be peformed atomically and idempotently,
>>>> which
>>>> is an advantage on the kexec path which can reenter acpi_dmar_reinstate().
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>> CC: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>>>> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
>>> Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>
>>>
>>> and curious do you observe a real atomic issue in kexec or just catch this
>>> potential issue when reading code? :-)
>> I have run over it once in the past, but mainly it is one small thing on
>> a very long list of tweaks to make the crash path for reliable.
>>
>> As indicated in the other thread, I think the best direction moving
>> forwards is to see about positively preventing dom0 having access,
>> rather than simply hiding the table, but that is a job for another time.
> And possibly not doable, as this might crash Dom0. What made me
> wonder for a very long time though is why similar clobbering isn't
> needed for AMD.

Any dom0 driver will be capable of not crashing if it can't get to
certain pages, or it wouldn't last for any meaningful time on a system
with buggy firmware.  It is the very fact that this hack is only used on
Intel which leads me to suspect that it is the wrong thing to be doing
overall.

>
> In any event, David's point of the now chosen signature perhaps
> posing a higher risk of colliding with a real table is an issue that
> shouldn't have been discarded before committing.

I don't believe the new name is plausibly at a higher risk of colliding.

> Unless Kevin or
> Yang object, I'd therefore suggest reverting the change. Once
> we determined why VT-d needs what AMD Vi doesn't need, and
> once we settled on the risk of name collision (perhaps using an
> underscore prefixed name would further reduce this risk), we could
> then do this another way (zap the table from XSDT/RSDT instead?),
> or leave it as it was without the change.

It is my hope that this can be resolved in the longterm without any
modification to the acpi tables.  Currently, it is not possible to dump
the ACPI tables from dom0 without knowing how to hexedit the XMAR table
back into life.  This is an impediment to debugging.

However, I still believe that the current change is a positive
improvement over what happened previously.

>
> (Apart from the above I also don't really see why RMAD was
> chosen - this doesn't really resemble anything similar to DMAR
> except for using the same letters. If at least it had been the
> properly reversed string ...)

A fully reversed string is RAMD which I felt was slightly more likely to
collide, but I am not too fussed on exactly which string is chosen, so
long as it has the same u8 checksum as "DMAR".

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel