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]

 





On 8/5/2024 1:30 PM, Michael Kelley wrote:
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.

Understood! I made an assumption that's just an array of random bytes.
Thank you very much for explaining that to me :)

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. :-)

Agreed, thanks for the discussion! I've discussed changing the values,
and a pull request to the hypervisor has been put up for that. So far, going well.

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.

Will be a badge of honor to me! Again, appreciate your words of advise and caution, and helping in bringing these patches into the best shape possible!

Michael

--
Thank you,
Roman





[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