On 29.03.2011, at 18:46, Scott Wood wrote: > On Tue, 29 Mar 2011 08:09:09 -0500 > Jimi Xenidis <jimix@xxxxxxxxx> wrote: > >> >> On Mar 29, 2011, at 4:18 AM, Alexander Graf wrote: >> >>> >>> On 28.03.2011, at 21:15, Scott Wood wrote: >>> >>>> Return the actual host SVR. On e500, qemu currently pretends in the device >>>> tree to be an mpc8544 regardless of what the host is, but that's something >>>> that ought to be changed. >>> >>> Let's please keep the abstraction layers intact. I really don't want to see any real hardware information leak through inside emulate.c. SVR should be implemented the same way as PVR. >>> >>> Also, I don't think the register is actually meaningful on book3s. So it'd be good to move it to BookE specific code. >> >> It certainly not part of the powerpc 4xx core or the Book3E ISA, so please define it accordingly. > > Sorry, this should have gone in e500_emulate.c. > > As for letting real hardware info leak through, this is how PVR is handled > on e500 as well in the existing code -- and for the use case of directly Yes and no. You probably mean this line: arch/powerpc/kvm/e500.c: vcpu->arch.pvr = mfspr(SPRN_PVR); While it's true that for now the pvr is read out on initialization time from the real CPU, the main reason behind it is that we haven't established an interface that allows us to set it properly. We still maintain the abstraction by at least keeping it in a vcpu struct variable. > assigning peripherals to guests (which isn't supported upstream yet, but > we've got it running locally (more patches to come...), and it's what our > customers are interested in), you really don't want to confuse the guest > about what hardware it's running on. Yup - but we don't want to block the doors on it either :). > OTOH, with Qemu-emulated devices we want the guest to know what chip's > quirks are being emulated. It gets ugly if you want some directly-assigned > devices and some emulated devices... > > Even when not assigning devices directly, some "leakage" is inevitable. > E.g. SPE, normal FPU, neither? Errata? e500mc features (other than E.HV)? Oh, sure. We can't maintain all combinations in the abstraction. But I really don't want us to just stupidly pass every host bit through to the guest. If we can abstract it without major overhead, we should certainly do so. > Later in the patchset I'm working through for upstream is sregs support, > including SVR -- we can do Qemu-supplied PVR/SVR then, if needed. I think > the default sregs should have the real PVR/SVR, so Qemu can read it and > try to make its emulated devices behave consistently with it, if it's not a > situation where telling the guest it's running on some completely different > hardware is going to work well. If Qemu wants to override instead, it can. That's where we should be heading, right. So if you simply change this patch to create a vcpu->arch.svr field and assign that the same way we do for pvr, you already did a good step towards the qemu-supplied version, as you won't have to touch the emulation code again later. > Note that the current situation without this patch is that the guest kernel > fails to boot when the gianfar driver checks SVR for errata. Understood. And I'm fine with reading the SVR on init, so we don't require qemu changes for things to get working again. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
