On Mon, Jun 17, 2019 at 12:06:32PM +1000, Paul Mackerras wrote: > On Thu, Jun 06, 2019 at 02:36:09PM -0300, Claudio Carvalho wrote: > > From: Ram Pai <linuxram@xxxxxxxxxx> > > > > Add the ucall() function, which can be used to make ultravisor calls > > with varied number of in and out arguments. Ultravisor calls can be made > > from the host or guests. > > > > This copies the implementation of plpar_hcall(). > > One point which I missed when I looked at this patch previously is > that the ABI that we're defining here is different from the hcall ABI > in that we are putting the ucall number in r0, whereas hcalls have the > hcall number in r3. That makes ucalls more like syscalls, which have > the syscall number in r0. So that last sentence quoted above is > somewhat misleading. > > The thing we need to consider is that when SMFCTRL[E] = 0, a ucall > instruction becomes a hcall (that is, sc 2 is executed as if it was > sc 1). In that case, the first argument to the ucall will be > interpreted as the hcall number. Mostly that will happen not to be a > valid hcall number, but sometimes it might unavoidably be a valid but > unintended hcall number. > > I think that will make it difficult to get ucalls to fail gracefully > in the case where SMF/PEF is disabled. It seems like the assignment > of ucall numbers was made so that they wouldn't overlap with valid > hcall numbers; presumably that was so that we could tell when an hcall > was actually intended to be a ucall. However, using a different GPR > to pass the ucall number defeats that. Right this is a valid point. Glad that you caught it, otherwise it would have become a difficult to fix it in the future. > > I realize that there is ultravisor code in development that takes the > ucall number in r0, and also that having the ucall number in r3 would > possibly make life more difficult for the place where we call > UV_RETURN in assembler code. Its called from one place in the hypervisor, and the changes look simple. - LOAD_REG_IMMEDIATE(r0, UV_RETURN) + LOAD_REG_IMMEDIATE(r3, UV_RETURN) ld r7, VCPU_GPR(R7)(r4) ld r6, VCPU_GPR(R6)(r4) ld r4, VCPU_GPR(R4)(r4) What am i missing? > Nevertheless, perhaps we should consider > changing the ABI to be like the hcall ABI before everything gets set > in concrete. yes. Thanks Paul! RP