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

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hello, Roger, Jan!

On 13.01.22 15:38, Jan Beulich wrote:
> 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.
That is by intention: we do not want to have this enabled on Arm until
it can really be used...
> 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.
...so we enable vPCI on Arm right after we are all set
> Then again there were at least 3 series in parallel originally, with
> interdependencies (iirc) not properly spelled out ...
Sorry about that, we should have said that explicitly
>
> Jan
>
Thank you,
Oleksandr