Re: [PATCH v5 14/14] vpci: add TODO for the registers not explicitly handled

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.01.2022 14:27, Roger Pau Monné wrote:
> On Thu, Nov 25, 2021 at 12:17:32PM +0100, Jan Beulich wrote:
>> On 25.11.2021 12:02, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>>
>>> For unprivileged guests vpci_{read|write} need to be re-worked
>>> to not passthrough accesses to the registers not explicitly handled
>>> by the corresponding vPCI handlers: without fixing that passthrough
>>> to guests is completely unsafe as Xen allows them full access to
>>> the registers.
>>>
>>> Xen needs to be sure that every register a guest accesses is not
>>> going to cause the system to malfunction, so Xen needs to keep a
>>> list of the registers it is safe for a guest to access.
>>>
>>> For example, we should only expose the PCI capabilities that we know
>>> are safe for a guest to use, i.e.: MSI and MSI-X initially.
>>> The rest of the capabilities should be blocked from guest access,
>>> unless we audit them and declare safe for a guest to access.
>>>
>>> As a reference we might want to look at the approach currently used
>>> by QEMU in order to do PCI passthrough. A very limited set of PCI
>>> capabilities known to be safe for untrusted access are exposed to the
>>> guest and registers need to be explicitly handled or else access is
>>> rejected. Xen needs a fairly similar model in vPCI or else none of
>>> this will be safe for unprivileged access.
>>>
>>> Add the corresponding TODO comment to highlight there is a problem that
>>> needs to be fixed.
>>>
>>> Suggested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>
>> Looks okay to me in principle, but imo needs to come earlier in the
>> series, before things actually get exposed to DomU-s.
> 
> Are domUs really allowed to use this code? Maybe it's done in a
> separate series, but has_vpci is hardcoded to false on Arm, and
> X86_EMU_VPCI can only be set for the hardware domain on x86.

I'm not sure either. This series gives the impression of exposing things,
but I admit I didn't pay attention to has_vpci() being hardcoded on Arm.
Then again there were at least 3 series in parallel originally, with
interdependencies (iirc) not properly spelled out ...

Jan