Re: [Qemu-devel] [PATCH 1/1] xen-hvm.c: Add support for Xen access to vmport

[Don Slutz <dslutz@verizon.com>]

On 09/29/14 07:53, Alexander Graf wrote:
>
> On 29.09.14 13:10, Paolo Bonzini wrote:
>> Il 29/09/2014 10:12, Alexander Graf ha scritto:
>>> Could you instead plug into the existing cpu_synchronize_registers()
>>> framework and just implement register synchronization for the Xen side,
>>> just like it's been done for KVM? :)
>> No, because here it's Xen that sends out the register contents.  With
>> KVM, it's QEMU that requests the register contents.
> So? We could still reuse the same infrastructure:
>
>
>    cpu_handle_ioreq()
>    {
>      ...
>
>      if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) {
>          cpu->xen_vcpu_dirty = true;

I am assuming that you mean something like:

     X86CPU *cpu = X86_CPU(current_cpu);
     cpu->xen_vcpu_dirty = true;

because cpu is not defined here.  And this has a major issue
because current_cpu is NULL for Xen, so you have added a SIGSEGV.

If you are trying to say "lets build a CPUState state object here
and set current_cpu to it, and then set the xen_vcpu_dirty" then
yes that could be done.  Adding all the code to build a CPUState state
object and X86CPUClass object correctly just to use the code in
vmport.c looked to me to be a lot of work for little benefit.


If you want to go with not adding the objects, I could change
to having a "xen X86CPU" that I set current_cpu to and
then back to NULL when done.  That (I think) would drop
all changes to vmport.c

>          synchronize_xen_to_env(xenptr, cpu);
>      }
>
>      handle_ioreq();
>
>      if (req->type == IOPORT_REGISTERS_SYNCHRONIZED) {
>          cpu->xen_vcpu_dirty = false;
>          synchronize_env_to_xen(xenptr, cpu);
>      }
>
>      ...
>    }
>
>    void xen_cpu_synchronize_state(CPUState *cpu)
>    {
>      assert(cpu->xen_vcpu_dirty);
>    }
>
> Then no changes to the vmport code would be necessary and this problems
> where some code path wants to do direct access to registers
> automatically tells us that things are broken.

The SIGSEGV already does this.

     -Don Slutz

>
> Alex