Re: [PATCH v5 07/11] xen/domctl: Introduce XEN_DOMCTL_CDF_vpci flag

[Bertrand Marquis <Bertrand.Marquis@arm.com>]

Hi,

> On 14 Oct 2021, at 07:23, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 13.10.2021 22:41, Stefano Stabellini wrote:
>> On Wed, 13 Oct 2021, Jan Beulich wrote:
>>> On 13.10.2021 14:11, Bertrand Marquis wrote:
>>>>> On 13 Oct 2021, at 11:56, Roger Pau Monné <roger.pau@citrix.com> wrote:
>>>>> If vPCI for Arm on 4.16 is not going to be functional, why so much
>>>>> pressure in pushing those patches so fast? I understand the need to
>>>>> remove stuff from the queue, but I don't think it's worth the cost of
>>>>> introducing a broken interface deliberately on a release.
>>> 
>>> +1
>>> 
>>>> We did not push that fast, those have been on the mailing list for a while (this is the 5th version of the serie).
>>>> PCI passthrough is a big change requiring lots of patches and we decided to push it piece by piece to make
>>>> the review easier.
>>> 
>>> 5 versions for a series like this one was to be expected. Imo it has
>>> been wrong in the past to rush in new features (even if experimental
>>> ones) very close to the freeze, and it has mislead people to think
>>> they can delay work until very (too?) late a point in time.
>> 
>> 
>> Hi Roger, Jan,
>> 
>> Let me take a few minutes to clarify and provide context for this work.
>> 
>> 
>> I don't think anyone "pushed hard" any of the ARM series close to the
>> release. I sent "one" email in public as a reminder of things that need
>> reviewing:
>> https://marc.info/?l=xen-devel&m=163373776611154
>> 
>> I did send a couple of private emails to Jan but they were to synchronize
>> ourselves rather than push; Jan, I hope you didn't take them the wrong
>> way.
> 
> Definitely not, no worries.
> 
>> At the same time it is certainly true that all the people involved here
>> worked very hard to get these series ready for 4.16. Oct 15 is the Xen
>> Project feature freeze. It is also the deadline for many of us working
>> with upstream Xen Project to upstream features and report back to our
>> management. I know it doesn't affect you guys directly, but you can
>> imagine that as employees of our respective companies we feel pressure
>> to complete our objectives in the given timeframe. Of course we need to
>> make sure to do that without compromising quality and without
>> overextending upstream review capacity.
>> 
>> 
>> The ARM PCI series are public since Dec 2020, pushed to a gitlab branch
>> for testing: https://gitlab.com/xen-project/fusa/xen-integration/-/tree/integration/pci-passthrough
>> I helped creating, managing and testing the branch.
>> 
>> So, I can see why Bertrand feels like they have been around for a while:
>> we have been dealing with these patches for almost a year, even if from
>> a xen-devel perspective we are "only" at version 5.
> 
> I'm afraid that anything not on xen-devel doesn't really count; the list
> is the only "official" channel. ISTR that for certain aspects there's a
> plan George has to make this more pronounced / formal in some of the docs
> we have.
> 
> Making internally set deadlines is a fully understandable aspect. But
> starting to post in September for a mid-October deadline is putting
> oneself at risk, I would say, for a (set of) series this involved. I see
> no problem with anyone doing so, as long as they accept that risk rather
> than expecting to get away by either extraordinary efforts by others
> (reviewers) or relaxation of what is permitted to go in.
> 
> Of course there's the opposite problem of feedback taking unusually (I'm
> inclined to say unduly) long to arrive, like with the gpaddr_bits
> addition still facing vague / unclear opposition by Andrew.
> 
>> My suggestion is to accept the TODO for patch #8 [1] but I agree with
>> Roger that we shouldn't introduce bad interfaces, especially as they
>> impact x86 which is not "tech preview". So I think we should revert
>> patch #7 (this patch.) I'll reply with more details in separate shorter
>> email.
>> 
>> [1] https://marc.info/?l=xen-devel&m=163412120531248
> 
> FWIW I agree with both proposals (acceptance of TODO and revert).

I want to say again that nothing I said was meant as a critic and I am very
grateful of all reviews and comments that we have (even if some of them
are implying work or pushing stuff in the future) as we want to do this right.

We will do the following before the end of the day today:
- send a reverting patch for patch 7
- send updated version of patch 8 handling all pending comments

We will try to (not sure as some changes might be more complex)
- send an updated version of patch 10
- try to find a solution acceptable for patch 1

Thanks everyone for the support here and the huge amount of time
spent on reviewing all patches that was pushed (and there was a lot of them).

Over the next month Arm is committed to put a lot more effort in reviewing and
testing patches pushed to xen-devel. We now have an internal CI that will help
us a lot and all people in Arm Xen team will start to have dedicated time to review
upstream patches and help the community.
Xen is a big part of Arm strategy around automotive as the reference Type 1
hypervisor and we will not disappear soon :-)

If there are any outstanding serie or patches for which you would like us to help
On review, please do not hesitate to ping me and I will see how we can help (even
if it is only to do some testing).

Thanks again everyone for the help.

Cheers
Bertrand

PS: I will stay connected on IRC if someone needs to contact me.

> 
> Jan