Re: [PATCH 6/9] vpci/header: Handle p2m range sets per BAR

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 09.09.21 12:39, Jan Beulich wrote:
> On 09.09.2021 11:12, Oleksandr Andrushchenko wrote:
>> Anyways, I am open to any decision on what would be the right approach here:
>>
>> 1. Use range sets per BAR as in the patch
>>
>> 2. Remove range sets completely and have a per-vCPU list with mapping
>>
>> data as I described above
>>
>> 3. Anything else?
> A decision first requires a proposal.

I already have 2: one in the patch with the range set per BAR and one described

earlier in the thread with a single range set and a list for GFN <-> MFN.

If you can tell your opinion I am all ears. But, please be specific as common 
words

don't change anything to me.

At the same time I do understand that the current code is not set in stone,

but we should have a good reason for major changes to it, IMO. I mean that 
before

DomU's we were fine with the range sets etc, and now we are not:

so what has changed so much?

>   I think 3 is the way to investigate
> first: Rather than starting from the code we currently have, start from
> what you need for DomU-s to work. If there's enough overlap with how we
> handle Dom0, code can be shared.

You can see that in my patch the same code is used by both hwdom and

guest. What else needs to be proven? The patch shows that all the code

besides guest register handlers (which is expected) is all common.

>   If things are sufficiently different,
> separate code paths are likely better. As said - to me a guest altering a
> BAR is merely a very special form of a request to change its P2M. The M
> parts remains unchanged (which is the major difference from Dom0), while
> the P part changes. As long as you can assume no two BARs to share a page,
> this would appear to suggest that it's simply a P2M operation plus book
> keeping at the vPCI layer. Completely different from Dom0 handling.

Underneath, yes, possibly. But at the level vPCI operates there is no

such difference I can clearly see in vPCI code and the patch in question.

Please point me to the vPCI code I fail to see.

>
> All of this applies only with memory decoding enabled, I expect.
> Disabling memory decoding on a device ought to be a simple "unmap all
> BARs", while enabling is "map all BARs". Which again is, due to the
> assumed lack of sharing of pages, much simpler than on Dom0: You only
> need to subtract the MSI-X table range(s) (if any, and perhaps not
> necessary when unmapping, as there's nothing wrong to unmap a P2M slot
> which wasn't mapped); this may not even require any rangeset at all to
> represent.
>
> And in fact I wonder whether for DomU-s you want to support BAR changes
> in the first place while memory decoding is enabled.

No, why? I want to keep the existing logic, e.g. with memory decoding

disabled as it is now.

>   Depends much on
> how quirky the guest OSes are that ought to run on top.
>
> Jan
>
Thank you,

Oleksandr