On 5/7/20 5:51 PM, Laine Stump wrote:
On 5/6/20 1:48 PM, Andrea Bolognani wrote:
On Mon, 2020-04-20 at 21:55 +0200, Boris Fiuczynski wrote:
On 4/10/20 2:06 PM, Andrea Bolognani wrote:
On Thu, 2020-04-09 at 12:30 +0200, Shalini Chellathurai Saroja wrote:
The ZPCI address validation or autogeneration does not work as
expected in the following scenarios
1. uid = 0 and fid = 0
2. uid = 0 and fid > 0
3. uid = 0 and fid not specified
4. uid not specified and fid > 0
5. 2 ZPCI devices with uid > 0 and fid not specified.
This is because of the following reasons
1. If uid = 0 or fid = 0 the code assumes that user has not specified
the corresponding address
2. If either uid or fid is provided, the code assumes that both uid
and fid addresses are specified by the user.
I'd have to dig up the old threads, but based on what I remember the
behaviors you describe are entirely intentional.
For PCI addresses, setting all parts of the address to zero or not
setting it at all is equivalent, and we wanted to be consistent with
that behavior for ZPCI; additionally, zero is not a valid value for
uid so of course neither is the address uid=0 fid=0, which means that
we're not preventing the user from specifying a valid address by
conflating the all-zero address with the unspecified address.
For partially-specified addresses, the behavior is also the same as
PCI: any part you don't specify is considered to be zero, which
results in
uid=0 fid=0 -> uid=0 fid=0 -> address gets autogenerated
uid=0 fid=x -> uid=0 fid=x -> address is rejected as invalid
uid=0 -> uid=0 fid=0 -> address gets autogenerated
fid=x -> uid=0 fid=x -> address is rejected as invalid
uid=x -> uid=x fid=0 -> address is accepted
So, just like for PCI addresses, you have basically two reasonable
options: either don't specify any zPCI address and leave allocation
entirely up to libvirt, or specify all of the addresses completely:
anything in between will likely not work as you'd expect or want.
Again, this is based purely on my recollection of design discussions
that happened one and a half years ago, so I might have gotten some
of the details wrong - in which case by all means call me out on
that O:-)
Hi Andrea,
sorry for the delayed answer. I (and some others as well) lost some
emails on my IMAP account and I just found your answer today.
No apologies needed: I also took a long time to reply to your
message, and in my case there's no mail server malfunction that I
can assign the blame to O:-)
I can remember that you had a discussion with the original author of the
zpci code. There are a few issues with the currently implemented "rules"
which partially are not even working as you outlined above in more
complex scenarios.
I disagree with this assessment - they work exactly as designed and
as described above. Whether we *want* them to behave that way... Now
that's a different topic :)
I think the disconnect lies in what the user's expectations are and
what libvirt actually implements. Basically the user expects that
* if either one of uid and fid is explictly assigned a value by
the user, then the guest will use that value - unless such value
is invalid, in which case libvirt will report an error;
* if either one of uid and fid is absent from the user-provided
configuration, then libvirt will automatically pick a valid value
for the attribute.
This is not how the current zPCI implementation works, or how PCI
address assignment works in libvirt; on the other hand, I think these
expectations are in fact completely reasonable, as the examples you
provide illustrate quite well.
I think you successfully convinced me that the current approach is
not good for users and we should fix it; my only doubt at this point
is whether can we safely do that without breaking libvirt's backwards
compatibility guarantees.
Dan, Laine, what's your take?
It sounds like the same semantics were applied to the zPCI address
element as are applied to <address type='pci'> (if I understand
correctly, while an PCI address is considered "valid and complete" if
any of its attributes is non-0, for the zPCI extensions, each attribute
is taken on its own. Is that correct?
Yes, that is correct.
uid and fid can be set independently from each other within their ranges.
uid (a hex value between 0x0001 and 0xffff, inclusive)
fid (a hex value between 0x00000000 and 0xffffffff, inclusive)
Both must be unique within a domain.
In any case, it sounds like current behavior for zPCI is broken for some
use cases, and imo this is new enough and usage is narrow enough that if
the maintainers (who I would guess represent the actual users) think
fixing the bug is more important than 100% backward compatibility in a
corner case, then they should be able to change it.
I would like to get this fixed such that the behavior is architecture
compliant.
The behavior change would be
Current code:
uid=0 fid=0 -> uid=0 fid=0 -> address gets autogenerated
uid=0 fid=x -> uid=0 fid=x -> address is rejected as invalid
uid=0 -> uid=0 fid=0 -> address gets autogenerated
With the series applied
uid=0 fid=0 -> uid=0 fid=0 -> address is rejected as invalid
uid=0 fid=x -> uid=0 fid=x -> address is rejected as invalid
uid=0 -> uid=0 fid=0 -> address is rejected as invalid
The documentation already specifies the uid value range correctly.
The fix for users hitting the two scenarios (uid=0 fid=0) and (uid=0) is
simple: Remove the zpci definition completely.
My expectation is that most users not interested in defining the values
never specified the two scenarios anyway. So I agree with you that it is
an incompatible change in a corner case.
Now if you wanted to change the way regular PCI address attributes are
handled, *then* I would have a totally different opinion :-)
I am not asking for such change even so I have in the past asked about
strange behavior when specifying a slot... :-)
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
