On Wed, Mar 04, 2020 at 11:59:48AM +0100, Greg Kurz wrote: > On Tue, 3 Mar 2020 10:56:45 -0800 > Ram Pai <linuxram@xxxxxxxxxx> wrote: > > > On Tue, Mar 03, 2020 at 06:45:20PM +0100, Greg Kurz wrote: > > > On Tue, 3 Mar 2020 09:02:05 -0800 > > > Ram Pai <linuxram@xxxxxxxxxx> wrote: > > > > > > > On Tue, Mar 03, 2020 at 07:50:08AM +0100, Cédric Le Goater wrote: > > > > > On 3/3/20 12:32 AM, David Gibson wrote: > > > > > > On Fri, Feb 28, 2020 at 11:54:04PM -0800, Ram Pai wrote: > > > > > >> XIVE is not correctly enabled for Secure VM in the KVM Hypervisor yet. > > > > > >> > > > > > >> Hence Secure VM, must always default to XICS interrupt controller. > > > > > >> > > > > > >> If XIVE is requested through kernel command line option "xive=on", > > > > > >> override and turn it off. > > > > > >> > > > > > >> If XIVE is the only supported platform interrupt controller; specified > > > > > >> through qemu option "ic-mode=xive", simply abort. Otherwise default to > > > > > >> XICS. > > > > > > > > > > > > Uh... the discussion thread here seems to have gotten oddly off > > > > > > track. > > > > > > > > > > There seem to be multiple issues. It is difficult to have a clear status. > > > > > > > > > > > So, to try to clean up some misunderstandings on both sides: > > > > > > > > > > > > 1) The guest is the main thing that knows that it will be in secure > > > > > > mode, so it's reasonable for it to conditionally use XIVE based > > > > > > on that > > > > > > > > > > FW support is required AFAIUI. > > > > > > 2) The mechanism by which we do it here isn't quite right. Here the > > > > > > guest is checking itself that the host only allows XIVE, but we > > > > > > can't do XIVE and is panic()ing. Instead, in the SVM case we > > > > > > should force support->xive to false, and send that in the CAS > > > > > > request to the host. We expect the host to just terminate > > > > > > us because of the mismatch, but this will interact better with > > > > > > host side options setting policy for panic states and the like. > > > > > > Essentially an SVM kernel should behave like an old kernel with > > > > > > no XIVE support at all, at least w.r.t. the CAS irq mode flags. > > > > > > > > > > Yes. XIVE shouldn't be requested by the guest. > > > > > > > > Ok. > > > > > > > > > This is the last option > > > > > I proposed but I thought there was some negotiation with the hypervisor > > > > > which is not the case. > > > > > > > > > > > 3) Although there are means by which the hypervisor can kind of know > > > > > > a guest is in secure mode, there's not really an "svm=on" option > > > > > > on the host side. For the most part secure mode is based on > > > > > > discussion directly between the guest and the ultravisor with > > > > > > almost no hypervisor intervention. > > > > > > > > > > Is there a negotiation with the ultravisor ? > > > > > > > > The VM has no negotiation with the ultravisor w.r.t CAS. > > > > > > > > > > > > > > > 4) I'm guessing the problem with XIVE in SVM mode is that XIVE needs > > > > > > to write to event queues in guest memory, which would have to be > > > > > > explicitly shared for secure mode. That's true whether it's KVM > > > > > > or qemu accessing the guest memory, so kernel_irqchip=on/off is > > > > > > entirely irrelevant. > > > > > > > > > > This problem should be already fixed. > > > > > The XIVE event queues are shared > > > > > > > > Yes i have a patch for the guest kernel that shares the event > > > > queue page with the hypervisor. This is done using the > > > > UV_SHARE_PAGE ultracall. This patch is not sent out to any any mailing > > > > lists yet. > > > > > > Why ? > > > > At this point I am not sure if this is the only change, I need to the > > guest kernel. > > Maybe but we're already sure that this change is needed. I don't really see > the point in holding this any longer. > > > I also need changes to KVM and to the ultravisor. Its bit > > premature to send the patch without having figured out everything > > to get xive working on a Secure VM. > > > > I'm a bit confused... why did you send this workaround patch in > the first place then ? I mean, this raises a concern and we're > just trying to move forward. The upstream kernel in its current form, will hang as of today if qemu has 'ic-mode=xive'. The kernel hangs without giving any indication as to 'why?'. This is a bad state to be in. This patch was a temporary solution. It is there to inform the user, not to use 'xive' in secureVM. The user is atleast informed/guided, instead of lost/confused. The permanent solution, is to fix the problem in KVM and ultravisor, along with sharing the EQ-page in the SVM, and get a holistic solution in place. But that will take time, and may not happen by the time 5.6 releases. RP