Re: [Xen-devel] [PATCH v6 15/19] x86/VPMU: Add privileged PMU mode

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 05/26/2014 07:48 AM, Jan Beulich wrote:
> > +                gregs = guest_cpu_user_regs();
> > +                memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
> > +                       gregs, sizeof(struct cpu_user_regs));
> > +            }
> > +            else
> > +                memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
> > +                       regs, sizeof(struct cpu_user_regs));
> >   
> > -            cmp = (void *)&v->arch.vpmu.xenpmu_data->pmu.r.regs;
> > -            XLAT_cpu_user_regs(cmp, gregs);
> > -            memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
> > -                   &cmp, sizeof(struct compat_cpu_user_regs));
> > +            gregs = &v->arch.vpmu.xenpmu_data->pmu.r.regs;
> > +            gregs->cs = (current->arch.flags & TF_kernel_mode) ? 0 : 0x3;
> Ah, no - you want to modify the structure here. But you could do this
> directly on the ->pmu.r.regs field rather than first latching the pointer.
>
> And as said before, it doesn't really look correct to simply set ->cs to
> just the RPL, especially without any comment explaining why this is
> (a) being done and (b) correct.
The reason for only passing up the RPL is that's the only field that the 
guest is interested in (whether the interrupt happened in kernel or user 
space). I added a comment in the code to this effect.
Do you think that all fields need to be passed?

-boris


> >           }
> > -        else if ( !is_hardware_domain(current->domain) &&
> > -                 !is_idle_vcpu(current) )
> > +        else
> >           {
> > -            /* PV guest */
> > +            /* HVM guest */
> > +            struct segment_register cs;
> > +
> >               gregs = guest_cpu_user_regs();
> >               memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
> >                      gregs, sizeof(struct cpu_user_regs));
> > +
> > +            hvm_get_segment_register(current, x86_seg_cs, &cs);
> > +            gregs = &v->arch.vpmu.xenpmu_data->pmu.r.regs;
> > +            gregs->cs = cs.attr.fields.dpl;
> Same here then obviously.
>
> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel