RE: [PATCH v3 1/7] arm64: hyperv: Use SMC to detect hypervisor presence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Roman Kisel <romank@xxxxxxxxxxxxxxxxxxx> Sent: Monday, August 5, 2024 9:51 AM

[snip]

> >> diff --git a/arch/arm64/include/asm/mshyperv.h
> >> b/arch/arm64/include/asm/mshyperv.h
> >> index a975e1a689dd..a7a3586f7cb1 100644
> >> --- a/arch/arm64/include/asm/mshyperv.h
> >> +++ b/arch/arm64/include/asm/mshyperv.h
> >> @@ -51,4 +51,9 @@ static inline u64 hv_get_msr(unsigned int reg)
> >>
> >>   #include <asm-generic/mshyperv.h>
> >>
> >> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_0	0x7948734d
> >> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_1	0x56726570
> >> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_2	0
> >> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_3	0
> >> +
> >
> > Section 6.2 of the SMCCC specification says that the "Call UID Query"
> > returns a UUID. The above #defines look like an ASCII string is being
> > returned. Arguably the ASCII string can be treated as a set of 128 bits
> > just like a UUID, but it doesn't meet the spirit of the spec. Can Hyper-V
> > be changed to return a real UUID? While the distinction probably
> > won't make a material difference here, we've had problems in the past
> > with Hyper-V doing slightly weird things that later caused unexpected
> > trouble. Please just get it right. :-)
> >
> The above values don't violate anything in the spec, and I think it
> would be hard to give an example of what can be broken in the world by
> using the above values.

Agreed.  However, note that UUIDs *do* have some internal structure.
See https://www.uuidtools.com/decode, for example. The UUID above
is:

4d734879-7065-7256-0000-000000000000

It has a version digit of "7", which is "unknown".  I'm no expert on UUIDs,
but apparently the "7" isn't necessarily invalid, though perhaps a bit unexpected.

> I do understand what you're saying when you
> mention the UIDs & the spirit of the spec. Put on the quantitative
> footing, the Shannon entropy of these values is much lower than that of
> an UID. A cursory search in the kernel tree does turn up other UIDs that
> don't look too random.
> 
> As that is implemented only in the non-release versions, hardly someone
> has taken a dependency on the specific values in their production code.
> I guess that can be changed without causing any disturbance to the
> customers, will report of any concerns though.

My last thought is that when dealing with the open source world, and
with published standards, it's usually best to do what's normal and
expected, rather than trying to make the case that something unexpected
is allowed by the spec. Doing the latter can come back to bite you in
completely unexpected ways. :-)

With that, I won't make any further comments on the topic. You do
whatever you can do in working with Hyper-V. Either way, it won't be
a blocker to my giving "Reviewed-by" on the next version of the
patch.

Michael




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux