From: Roman Kisel <romank@linux.microsoft.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com, bhelgaas@google.com,
"Borislav Petkov" <bp@alien8.de>,
"Catalin Marinas" <catalin.marinas@arm.com>,
"Conor Dooley" <conor+dt@kernel.org>,
"Dave Hansen" <dave.hansen@linux.intel.com>,
"Dexuan Cui" <decui@microsoft.com>,
"Haiyang Zhang" <haiyangz@microsoft.com>,
"H. Peter Anvin" <hpa@zytor.com>,
krzk+dt@kernel.org, "Krzysztof Wilczyński" <kw@linux.com>,
"K. Y. Srinivasan" <kys@microsoft.com>,
"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
"Ingo Molnar" <mingo@redhat.com>, "Rob Herring" <robh@kernel.org>,
ssengar@linux.microsoft.com,
"Thomas Gleixner" <tglx@linutronix.de>,
"Wei Liu" <wei.liu@kernel.org>, "Will Deacon" <will@kernel.org>,
devicetree@vger.kernel.org,
Linux-Arch <linux-arch@vger.kernel.org>,
linux-arm-kernel@lists.infradead.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH hyperv-next v4 1/6] arm64: hyperv: Use SMCCC to detect hypervisor presence
Date: Tue, 25 Feb 2025 14:25:24 -0800 [thread overview]
Message-ID: <a96f9469-a22e-43e7-825d-f67ef550898f@linux.microsoft.com> (raw)
In-Reply-To: <55b65ba6-4abe-478c-a173-4622c30ddd7b@app.fastmail.com>
On 2/24/2025 11:24 PM, Arnd Bergmann wrote:
> On Tue, Feb 25, 2025, at 00:22, Roman Kisel wrote:
>> Hi Arnd,
[...]
> If you want to declare a uuid here, I think you should remove the
> ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_{0,1,2,3} macros and just
> have UUID in normal UUID_INIT() notation as we do for
> other UUIDs.
I'd gladly stick to that provided I have your support of touching
KVM's code! As the SMCCC document states, there shall be an UUID,
and in the kernel, there would be
#define ARM_SMCCC_VENDOR_KVM_UID UUID_INIT(.......)
#define ARM_SMCCC_VENDOR_HYP_UID UUID_INIT(.......)
Hence, the ARM_SMCCC_VENDOR_HYP_UID_*_REG_{0,1,2,3} can be removed as
you're suggesting.
That looks enticing enough semantically as though we're building layers
from the SMCCC spec down to the "on-wire format" -- the only part that
needs "deserializing" the UUID from `struct arm_smccc_res` the
hypervisor returns.
To add to that, anyone who wishes to implement a hypervisor for arm64
will have to use some RFC 9562-compliant UUID generating facility. Thus,
the UUID predates these 4 dwords. Using UUIDs in the kernel code will
relieve of the chore of figuring out the 4 dwords from the UUID.
Also, for the Gunyah folks will be pretty easy to use this infra:
define the UUID in the header (1 line), call the new function (1 line),
done.
>
> If you want to keep the four 32-bit values and pass them into
> arm_smccc_hyp_present() directly, I think that is also fine,
> but in that case, I would try to avoid calling it a UUID.
IMO, that approach provides a simplicity where anyone can see if the
code is wrong from a quick glance: just compare 4 dwords. The fact that
the 4 dwords form an UUID is bypassed though (as it is in the existing
code). Somehow feels not spec-accurate imo. Also when I remove the UID
part from the names, I'm going to have a rather weak justification as
to why this is a benefit.
Likely, there are two levels of improvement here:
1. Just refactor the common parts out and have
`bool arm_smccc_hyp_present(u32 reg0, u32 reg1, u32 reg2, u32 reg2);`
2. Introduce the UUID usage throughout and have a spec-accurate
prototype of
`bool arm_smccc_hyp_present(const uuid_t *hyp_uuid);`
and would be great to go for the second one :)
>
> How are the kvm and hyperv values specified originally?
>>From the SMCCC document it seems like they are meant to be
> UUIDs, so I would expect them to be in canonical form rather
> than the smccc return values, but I could not find a document
> for them.
For hyperv case, `uuidgen` produced the UUID and that is used.
Likely the same for kvm.
>
> Arnd
--
Thank you,
Roman
next prev parent reply other threads:[~2025-02-25 22:25 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-12 1:43 [PATCH hyperv-next v4 0/6] arm64: hyperv: Support Virtual Trust Level Boot Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 1/6] arm64: hyperv: Use SMCCC to detect hypervisor presence Roman Kisel
2025-02-12 6:54 ` Arnd Bergmann
2025-02-13 23:23 ` Roman Kisel
2025-02-14 8:05 ` Arnd Bergmann
2025-02-14 16:47 ` Roman Kisel
2025-02-24 23:22 ` Roman Kisel
2025-02-25 7:24 ` Arnd Bergmann
2025-02-25 22:25 ` Roman Kisel [this message]
2025-02-26 13:57 ` Arnd Bergmann
2025-02-19 23:13 ` Michael Kelley
2025-02-20 16:34 ` Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 2/6] Drivers: hv: Enable VTL mode for arm64 Roman Kisel
2025-02-19 23:14 ` Michael Kelley
2025-02-20 16:36 ` Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 3/6] Drivers: hv: Provide arch-neutral implementation of get_vtl() Roman Kisel
2025-02-19 23:17 ` Michael Kelley
2025-02-12 1:43 ` [PATCH hyperv-next v4 4/6] dt-bindings: microsoft,vmbus: Add GIC and DMA coherence to the example Roman Kisel
2025-02-12 6:42 ` Krzysztof Kozlowski
2025-02-12 23:57 ` Roman Kisel
2025-02-13 20:50 ` Roman Kisel
2025-02-12 1:43 ` [PATCH hyperv-next v4 5/6] Drivers: hv: vmbus: Get the IRQ number from DeviceTree Roman Kisel
2025-02-19 23:20 ` Michael Kelley
2025-02-12 1:43 ` [PATCH hyperv-next v4 6/6] PCI: hv: Get vPCI MSI IRQ domain " Roman Kisel
2025-02-12 17:42 ` Bjorn Helgaas
2025-02-18 22:32 ` Roman Kisel
2025-02-19 23:51 ` Roman Kisel
2025-02-19 23:29 ` Michael Kelley
2025-02-20 16:41 ` Roman Kisel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a96f9469-a22e-43e7-825d-f67ef550898f@linux.microsoft.com \
--to=romank@linux.microsoft.com \
--cc=arnd@arndb.de \
--cc=benhill@microsoft.com \
--cc=bhelgaas@google.com \
--cc=bp@alien8.de \
--cc=bperkins@microsoft.com \
--cc=catalin.marinas@arm.com \
--cc=conor+dt@kernel.org \
--cc=dave.hansen@linux.intel.com \
--cc=decui@microsoft.com \
--cc=devicetree@vger.kernel.org \
--cc=haiyangz@microsoft.com \
--cc=hpa@zytor.com \
--cc=krzk+dt@kernel.org \
--cc=kw@linux.com \
--cc=kys@microsoft.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=mingo@redhat.com \
--cc=robh@kernel.org \
--cc=ssengar@linux.microsoft.com \
--cc=sunilmut@microsoft.com \
--cc=tglx@linutronix.de \
--cc=wei.liu@kernel.org \
--cc=will@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.