Re: [Xen-devel] [PATCH 3/3] pvh/dom0: whitelist PVH Dom0 ACPI tables

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/02/18 15:48, Roger Pau Monné wrote:
> On Tue, Feb 13, 2018 at 08:22:33AM -0700, Jan Beulich wrote:
>>>>> On 13.02.18 at 16:11, <roger.pau@xxxxxxxxxx> wrote:
>>> On Tue, Feb 13, 2018 at 06:41:14AM -0700, Jan Beulich wrote:
>>>>>>> On 13.02.18 at 12:27, <roger.pau@xxxxxxxxxx> wrote:
>>>>> On Tue, Feb 13, 2018 at 04:04:17AM -0700, Jan Beulich wrote:
>>>>>>>>> On 13.02.18 at 10:59, <roger.pau@xxxxxxxxxx> wrote:
>>>>>>> On Tue, Feb 13, 2018 at 02:29:08AM -0700, Jan Beulich wrote:
>>>>>>>>>>> On 08.02.18 at 13:25, <roger.pau@xxxxxxxxxx> wrote:
>>>>>>>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>>>>>> A change like this should not come without description, providing a
>>>>>>>> reason for the change. Otherwise how will someone wanting to
>>>>>>>> understand the change in a couple of years actually be able to
>>>>>>>> make any sense of it. This is in particular because I continue to be
>>>>>>>> not fully convinced that white listing is appropriate in the Dom0
>>>>>>>> case (and for the record I'm similarly unconvinced that black listing
>>>>>>>> is the best choice, yet obviously we need to pick on of the two).
>>>>>>> I'm sorry, I thought we agreed at the summit to convert this to
>>>>>>> whitelisting because it was likely better to simply not expose unknown
>>>>>>> ACPI tables to guests.
>>>>>> "to guests" != "to Dom0".
>>>>>>
>>>>>>> I guess the commit message could be something like:
>>>>>>>
>>>>>>> "The following list of whitelisted APIC tables are either known to work
>>>>>>> or don't require any resources to be mapped in either the IO or the
>>>>>>> memory space.
>>>>>> Even if the white listing vs black listing question wasn't still
>>>>>> undecided, I think we should revert the patch in favor of one
>>>>>> with a description. The one above might be fine with "ACPI" in
>>>>>> place of "APIC" as far as tables actively white listed are
>>>>>> concerned, but then it still remains open why certain tables
>>>>>> haven't been included. I'm in particular worried about various
>>>>>> APEI related tables, but invisibility of e.g. an IBFT could also
>>>>>> lead to boot problems.
>>>>> Regarding APEI I think ERST, EINJ and HEST could be passed through,
>>>>> BERT however requires that the BOOT Error Region is mapped into Dom0
>>>>> p2m.
>>>>>
>>>>> Since PVH Dom0 creation still ends up in a panic, I see no problem in
>>>>> adding those in follow up patches.
>>>>>
>>>>> IBFT also looks safe to pass through.
>>>> But you realize I've named only the few that came to mind
>>>> immediately?
>>> Sure, what I have in this patch is just the minimal set (plus a few
>>> others that seem completely fine) needed in order to boot on my two
>>> test boxes.
>>>
>>> I know we will certainly have to expand this, but I see no issue in
>>> adding them as we go, the more that this is all still unused.
>> Unused - sure. But how will we learn which ones we need to add?
> I already have a kind of drafted list, of which ones could be added,
> which ones need some handlers in order to make sure relevant areas are
> mapped and finally a list of tables that will never be exposed to
> Dom0.
>
> This is based on the tables currently known to Xen from the actblX.h
> headers, there are probably more in the wild, even ones not documented
> in http://www.uefi.org/acpi at all.
>
> I can cleanup and send that list.
>
>> Surely waiting for problem reports from the field is not an
>> acceptable model.

APCI tables are no different to CPUID values, or MSRs, etc.  Such
reports from the field would be missing features, not bugs.  (TBF, I
wouldn't even put IBFT in to begin with, because I'm not convinced the
IOMMU logic is good enough to work in the common case.  We still don't
account for ACS/ARI errata in the PLX bridges, and iommu=dom0-strict
mode breaks horribly on all hardware I've ever tried using it on.)

Dom0 should not be treated specially.  It just has a bit more hardware
and permissions by default.  The current PV interactions, and especially
the workarounds we've had to maintain the hypervisor, demonstrate
precisely why a whitelist approach is better than a blacklist.

This is all completely brand new work, so we've got the opportunity to
do things correctly from the beginning.  I'd much rather it takes longer
to do properly, than inheriting the same mistakes and problems that PV
dom0 has.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel