>>> On 26.05.15 at 19:50, <boris.ostrov...@oracle.com> wrote:
> On 05/26/2015 12:13 PM, Jan Beulich wrote:
>>>>> On 21.05.15 at 19:57, <boris.ostrov...@oracle.com> wrote:
>>>
>>> + * guest when PMU_CACHED bit in pmu_flags is set (which is done by the
>>> + * hypervisor during PMU interrupt). Hypervisor will read updated data in
>>> + * XENPMU_flush hypercall and clear PMU_CACHED bit.
>>> + */
>>> +struct xen_pmu_arch {
>>> + union {
>>> + /*
>>> + * Processor's registers at the time of interrupt.
>>> + * WO for hypervisor, RO for guests.
>>> + */
>>> + struct xen_pmu_regs regs;
>>> + /* Padding for adding new registers to xen_pmu_regs in the future
> */
>>> +#define XENPMU_REGS_PAD_SZ 64
>>> + uint8_t pad[XENPMU_REGS_PAD_SZ];
>>> + } r;
>>> +
>>> + /* WO for hypervisor, RO for guest */
>>> + uint64_t pmu_flags;
>>> +
>>> + /*
>>> + * APIC LVTPC register.
>>> + * RW for both hypervisor and guest.
>>> + * Only APIC_LVT_MASKED bit is loaded by the hypervisor into hardware
>>> + * during XENPMU_flush or XENPMU_lvtpc_set.
>>> + */
>>> + union {
>>> + uint32_t lapic_lvtpc;
>> Considering this isn't being used in this patch, could I ask you to
>> move it where it is being used (keeping only the pad member and
>> perhaps a placeholder comment here), so verifying that the
>> read-once requirement for the hypervisor side is met becomes
>> more obvious?
>
> I can certainly delay defining this field until later patch but how is
> this filed different from xen_pmu_arch (not seen here) which is also
> read-once? Wouldn't you then want to have that definition deferred as well?
I'm confused - the only xen_pmu_arch I see in this patch is
"struct xen_pmu_arch" (and its uses), which the field above is
actually part of, and which is also visible in the context above. So
I doubt that's what you're referring to. But yes, fields with read-
once requirements would better be defined in the patch(es) using
them, so reviewers don't need to hunt them down.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
