Re: [PATCH] riscv: Add initial Xen guest support for RISC-V

[Milan Đokić <milandjokic1995@xxxxxxxxx>]

On Thu, Jan 16, 2025 at 11:23 AM Radim Krčmář <rkrcmar@xxxxxxxxxxxxxxxx> wrote:
>
> 2025-01-16T09:51:25+01:00, Andrew Jones <ajones@xxxxxxxxxxxxxxxx>:
> > On Wed, Jan 15, 2025 at 08:04:05PM +0100, Milan Đokić wrote:
> >> On Tue, Jan 14, 2025 at 7:18 PM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> 
> >> wrote:
> >> > On Tue, Jan 14, 2025 at 05:09:36PM +0100, Milan Djokic wrote:
> >> > > +#define SBI_ECALL 0xE
> >> >
> >> > Shouldn't this be 0xA000007, i.e. the SBI firmware specific extension
> >> > for Xen. Otherwise why refer to SBI? Note, '0xE' is an invalid, legacy
> >> > extension ID in SBI.
> >> >
> >> Hypercall is triggered through SBI and we defined 0xE just as an
> >> SBI_ECALL ID on Xen side for hypercall handling (among other operation
> >> IDs), so we're not referring to some standard /legacy ID here, just
> >> utilizing SBI for hypercall handling.
> >
> > If the SBI specified EIDs and binary encoding aren't used, then the
> > hypercalls aren't "triggered through SBI", Xen is just doing its own
> > thing on an ecall. Xen doesn't have to implement SBI at all, but if
> > it wants to provide SBI services, as well as its own hypercalls, then
> > the hypercalls should be encoded in the same way as SBI functions and
> > an EID allowed by the SBI specification for hypervisor-specific
> > functions should be used. For Xen, that EID is already specified and
> > it's 0xA000007.
>
> SBI specifies a complete calling convention, but it's not necessary for
> binary compatibility.  SBI also aims to simplify caller API.
>
> Linux maintainers will want a good reason for introducing separate Xen
> SBI call functions/macros (linux already has sbi_ecall, so please try to
> use and potentially improve it), but the ECALL is guaranteed to be SBI
> compatible as long as a7=0xA000007.
>
> a7 is needed to denote Xen's extension space and the remaining
> input/output registers can be implemented in any way Xen wants.
Thanks for clarification. We have checked the SBI specification and
everything is clear now.
We'll fix this in the following patch revision.

BR,
Milan