Re: [Xen-devel] [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@xxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel