Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
On Tue, Nov 29, 2016 at 10:33:08AM -0500, Boris Ostrovsky wrote: > These registers (pm1a specifically) are not all specific to pm timer > and are accessed by non-pmtimer code (for example, sleep/power button > emulation). > > In addition to moving those regsters to struct hvm_domain rename > HVM save state structures and routines as well. > > No functional changes are introduced. > > (While this file is being modified, also add emacs mode style rune) > > Signed-off-by: Boris Ostrovsky > --- > Changes in v4: > * New patch > > tools/misc/xen-hvmctx.c| 4 +- Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
On 12/01/2016 11:29 AM, Andrew Cooper wrote:
> On 01/12/16 16:28, Boris Ostrovsky wrote:
>> On 12/01/2016 10:52 AM, Jan Beulich wrote:
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
/*
- * PM timer
+ * ACPI registers
*/
-struct hvm_hw_pmtimer {
+struct hvm_hw_acpi {
uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter
*/
uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */
uint16_t pm1a_en; /* PM1a_EVT_BLK.PM1a_EN: enable register */
};
-DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
+DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
>>> However much I appreciate this switch to a better name, I'm not
>>> convinced we can actually do this as easily: There's no
>>> __XEN_TOOLS__ guard anywhere in this file, and hence everything
>>> here is part of the stable ABI. I'm afraid you minimally will have to
>>> add interface version guards, retaining the old naming for old
>>> consumers.
>> Right, I haven't though about out-of-tree users. Should new fields
>> (added in patch 7) also be guarded?
> Be aware that my Hypervisor migration v2 plans (which follow the CPUID
> plans) will remove all of this (as it should never have gotten into the
> ABI to start with), and replace it with something looking suspiciously
> like the other migration v2 stream formats.
>
> If you can get away without changing names for now, probably best to
> just leave comment.
OK, I can leave it as pmtimer then.
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
On 01/12/16 16:28, Boris Ostrovsky wrote:
> On 12/01/2016 10:52 AM, Jan Beulich wrote:
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>>
>>>
>>> /*
>>> - * PM timer
>>> + * ACPI registers
>>> */
>>>
>>> -struct hvm_hw_pmtimer {
>>> +struct hvm_hw_acpi {
>>> uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter
>>> */
>>> uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */
>>> uint16_t pm1a_en; /* PM1a_EVT_BLK.PM1a_EN: enable register */
>>> };
>>>
>>> -DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
>>> +DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
>> However much I appreciate this switch to a better name, I'm not
>> convinced we can actually do this as easily: There's no
>> __XEN_TOOLS__ guard anywhere in this file, and hence everything
>> here is part of the stable ABI. I'm afraid you minimally will have to
>> add interface version guards, retaining the old naming for old
>> consumers.
>
> Right, I haven't though about out-of-tree users. Should new fields
> (added in patch 7) also be guarded?
Be aware that my Hypervisor migration v2 plans (which follow the CPUID
plans) will remove all of this (as it should never have gotten into the
ABI to start with), and replace it with something looking suspiciously
like the other migration v2 stream formats.
If you can get away without changing names for now, probably best to
just leave comment.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
On 12/01/2016 10:52 AM, Jan Beulich wrote:
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>
>>
>> /*
>> - * PM timer
>> + * ACPI registers
>> */
>>
>> -struct hvm_hw_pmtimer {
>> +struct hvm_hw_acpi {
>> uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
>> uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */
>> uint16_t pm1a_en; /* PM1a_EVT_BLK.PM1a_EN: enable register */
>> };
>>
>> -DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
>> +DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
> However much I appreciate this switch to a better name, I'm not
> convinced we can actually do this as easily: There's no
> __XEN_TOOLS__ guard anywhere in this file, and hence everything
> here is part of the stable ABI. I'm afraid you minimally will have to
> add interface version guards, retaining the old naming for old
> consumers.
Right, I haven't though about out-of-tree users. Should new fields
(added in patch 7) also be guarded?
-boris
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
>>> On 29.11.16 at 16:33, wrote:
> @@ -108,12 +111,12 @@ static void pmt_update_time(PMTState *s)
> s->last_gtime = curr_gtime;
>
> /* Update timer value atomically wrt lock-free reads in handle_pmt_io().
> */
> -*(volatile uint32_t *)&s->pm.tmr_val = tmr_val;
> +*(volatile uint32_t *)&acpi->tmr_val = tmr_val;
Please use write_atomic() instead of retaining this casting.
> @@ -197,7 +202,7 @@ static int handle_evt_io(
> }
> else /* p->dir == IOREQ_READ */
> {
> -data = s->pm.pm1a_sts | (((uint32_t) s->pm.pm1a_en) << 16);
> +data = acpi->pm1a_sts | (((uint32_t) acpi->pm1a_en) << 16);
Please drop the stray blank and parentheses.
> @@ -237,16 +243,17 @@ static int handle_pmt_io(
> * updated value with a lock-free atomic read.
> */
> spin_barrier(&s->lock);
> -*val = read_atomic(&s->pm.tmr_val);
> +*val = read_atomic(&(acpi->tmr_val));
Please don't add unnecessary parentheses.
> @@ -261,21 +268,21 @@ static int pmtimer_save(struct domain *d,
> hvm_domain_context_t *h)
> x = (((s->vcpu->arch.hvm_vcpu.guest_time ?: hvm_get_guest_time(s->vcpu))
> -
>s->last_gtime) * s->scale) >> 32;
> if ( x < 1UL<<31 )
> -s->pm.tmr_val += x;
> -if ( (s->pm.tmr_val & TMR_VAL_MSB) != msb )
> -s->pm.pm1a_sts |= TMR_STS;
> + acpi->tmr_val += x;
Hard tab.
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -525,16 +525,16 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>
>
> /*
> - * PM timer
> + * ACPI registers
> */
>
> -struct hvm_hw_pmtimer {
> +struct hvm_hw_acpi {
> uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
> uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */
> uint16_t pm1a_en; /* PM1a_EVT_BLK.PM1a_EN: enable register */
> };
>
> -DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
> +DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
However much I appreciate this switch to a better name, I'm not
convinced we can actually do this as easily: There's no
__XEN_TOOLS__ guard anywhere in this file, and hence everything
here is part of the stable ABI. I'm afraid you minimally will have to
add interface version guards, retaining the old naming for old
consumers.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
