[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/6] target-ppc: Synchronize more SPR
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/6] target-ppc: Synchronize more SPRs to KVM using ONE_REG interface |
Date: |
Tue, 29 Jan 2013 12:32:24 +1100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Jan 25, 2013 at 12:24:06PM +0100, Alexander Graf wrote:
> On 25.01.2013, at 03:39, David Gibson wrote:
> > On Thu, Jan 24, 2013 at 05:32:42PM +0100, Alexander Graf wrote:
[snip]
> >>> +static void kvm_get_one_spr(CPUState *cs, uint64_t id, int spr)
> >>> +{
> >>> + PowerPCCPU *cpu = POWERPC_CPU(cs);
> >>> + CPUPPCState *env = &cpu->env;
> >>> + union {
> >>> + uint32_t u32;
> >>> + uint64_t u64;
> >>> + } val;
> >>> + struct kvm_one_reg reg = {
> >>> + .id = id,
> >>> + .addr = (uintptr_t) &val,
> >>> + };
> >>> + int ret;
> >>> +
> >>> + ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®);
> >>> + if (ret != 0) {
> >>> + fprintf(stderr, "Warning: Unable to retrieve SPR %d from KVM:
> >>> %s\n",
> >>> + spr, strerror(errno));
> >>
> >> Any way you could make this a WARN_ONCE style message?
> >
> > Uh.. I guess. Afaict, qemu doesn't have any infrastructure for that,
> > so I'd have to roll my own.
>
> As I stated in a later patch review, I think it makes sense to just
> not print anything by default.
>
> Usually, you would do
>
> if (cap_foo) {
> get_one_spr(...);
> }
>
> right? With one_reg I consciously omitted caps for every register,
> because you can just as well read and push the reg itself and thus
> know whether it works.
>
> So if you translate that to the above code, it would mean
>
> get_one_spr(...);
>
> without error message would be semantically identical to the first
> version.
>
> It would however make sense to have a debug print version in case
> you want to know why something goes wrong. It might also make sense
> to have an initial get_one_spr() or so for a FULL register push to
> warn the user that live migration will not work on his machine.
So, I realised I need to rework this substantially anyway. As you
say, I need to not attempt to sync these various SPRs for BookE. But
more generally, different CPUs will have a different set of supported
SPRs.
I have a draft in the works that adds a ONE_REG id code to the
ppc_spr_t structure, which is (optionally) initialized by
spr_register() calls in the cpu initialization. Then the kvm call
goes through the SPR table and just syncs all those with registered
ONE_REG ids. I'm at linux.conf.au this week, so I don't expect to
finish that off until next week. There are some uglies in that that I
still need to see if can be resolved yet.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: Digital signature
- [Qemu-devel] [0/6] Revised outstanding pseries patches, David Gibson, 2013/01/23
- [Qemu-devel] [PATCH 6/6] pseries: Adjust default VIO address allocations to play better with libvirt, David Gibson, 2013/01/23
- [Qemu-devel] [PATCH 1/6] target-ppc: Give a meaningful error if too many threads are specified, David Gibson, 2013/01/23
- [Qemu-devel] [PATCH 5/6] target-ppc: Synchronize VPA state with KVM, David Gibson, 2013/01/23
- [Qemu-devel] [PATCH 2/6] pseries: Improve handling of multiple PCI host bridges, David Gibson, 2013/01/23
- [Qemu-devel] [PATCH 3/6] target-ppc: Synchronize more SPRs to KVM using ONE_REG interface, David Gibson, 2013/01/23
[Qemu-devel] [PATCH 4/6] target-ppc: Synchronize FPU state with KVM, David Gibson, 2013/01/23
Re: [Qemu-devel] [0/6] Revised outstanding pseries patches, Alexander Graf, 2013/01/24