[PATCH v4 00/15] PVH VCPU hotplug support

[PATCH v4 00/15] PVH VCPU hotplug support

[Boris Ostrovsky]

This series adds support for ACPI-based VCPU hotplug for unprivileged
PVH guests.

Changes in v4:
* Replaced XEN_DOMCTL_set_avail_vcpus with XEN_DOMCTL_acpi_access,
  toolstack is expected to issue two ACPI "writes" to trigger the hotplug
* Moved pm1a registers from PMTState to hvm_domain. gpe0 registers are
  added there.
* Both pm1a and gpe0 registers are now represented as uint16_t for status
  and enable (and as result no new public macros for those registers' length
  are needed). Ths is partly to avoid changes in HVM code (i.e. pmtimer)
  and partly to simplify code. I don't expect larger registers will be
  needed any time soon.
* ACPI handling now lives in xen/arch/x86/hvm/acpi.c
* Moved definition of XEN_ACPI_CPU_MAP/XEN_GPE0_CPUHP_BIT from ioreq.h
  to public/arch-x86/xen.h. This is the best I could come up with but
  ioreq is the wrong place, now that everything happens in acpi.c
* Reordered some patches


Boris Ostrovsky (15):
  x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
  acpi: Make pmtimer optional in FADT
  acpi: Power and Sleep ACPI buttons are not emulated for PVH guests
  acpi: PVH guests need _E02 method
  acpi/x86: Define ACPI IO registers for PVH guests
  domctl: Add XEN_DOMCTL_acpi_access
  pvh/acpi: Install handlers for ACPI-related PVH IO accesses
  pvh/acpi: Handle ACPI accesses for PVH guests
  x86/domctl: Handle ACPI access from domctl
  events/x86: Define SCI virtual interrupt
  pvh: Send an SCI on VCPU hotplug event
  tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug
  pvh: Set online VCPU map to avail_vcpus
  pvh/acpi: Save ACPI registers for PVH guests
  docs: Describe PVHv2's VCPU hotplug procedure

 docs/misc/hvmlite.markdown             |  13 ++
 tools/firmware/hvmloader/util.c        |   4 +-
 tools/libacpi/build.c                  |   7 +
 tools/libacpi/libacpi.h                |   2 +
 tools/libacpi/mk_dsdt.c                |  17 ++-
 tools/libacpi/static_tables.c          |   4 +
 tools/libxc/include/xenctrl.h          |  20 +++
 tools/libxc/xc_domain.c                |  36 ++++++
 tools/libxl/libxl.c                    |   8 +-
 tools/libxl/libxl_arch.h               |   4 +
 tools/libxl/libxl_arm.c                |   6 +
 tools/libxl/libxl_x86.c                |  21 +++
 tools/libxl/libxl_x86_acpi.c           |   6 +-
 tools/misc/xen-hvmctx.c                |   4 +-
 xen/arch/x86/domctl.c                  |   9 ++
 xen/arch/x86/hvm/Makefile              |   1 +
 xen/arch/x86/hvm/acpi.c                | 225 +++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/hvm.c                 |   2 +
 xen/arch/x86/hvm/pmtimer.c             |  75 +++++++----
 xen/common/domctl.c                    |   5 +
 xen/common/event_channel.c             |   7 +-
 xen/include/asm-x86/domain.h           |   2 +
 xen/include/asm-x86/hvm/domain.h       |   6 +
 xen/include/asm-x86/hvm/vpt.h          |   1 -
 xen/include/public/arch-x86/hvm/save.h |   8 +-
 xen/include/public/arch-x86/xen.h      |  14 +-
 xen/include/public/domctl.h            |  25 ++++
 xen/include/xen/domain.h               |   1 +
 xen/include/xen/event.h                |   8 ++
 xen/include/xen/sched.h                |   3 +
 30 files changed, 498 insertions(+), 46 deletions(-)
 create mode 100644 xen/arch/x86/hvm/acpi.c


-- 
2.7.4


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

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

[Boris Ostrovsky]

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 <boris.ostrovsky@oracle.com>
---
Changes in v4:
* New patch

 tools/misc/xen-hvmctx.c                |  4 +-
 xen/arch/x86/hvm/pmtimer.c             | 67 +++++++++++++++++++++-------------
 xen/include/asm-x86/hvm/domain.h       |  2 +
 xen/include/asm-x86/hvm/vpt.h          |  1 -
 xen/include/public/arch-x86/hvm/save.h |  6 +--
 5 files changed, 49 insertions(+), 31 deletions(-)

diff --git a/tools/misc/xen-hvmctx.c b/tools/misc/xen-hvmctx.c
index 32be120..8e8a245 100644
--- a/tools/misc/xen-hvmctx.c
+++ b/tools/misc/xen-hvmctx.c
@@ -342,7 +342,7 @@ static void dump_hpet(void)
 
 static void dump_pmtimer(void)
 {
-    HVM_SAVE_TYPE(PMTIMER) p;
+    HVM_SAVE_TYPE(ACPI) p;
     READ(p);
     printf("    ACPI PM: TMR_VAL 0x%x, PM1a_STS 0x%x, PM1a_EN 0x%x\n", 
            p.tmr_val, (unsigned) p.pm1a_sts, (unsigned) p.pm1a_en);
@@ -462,7 +462,7 @@ int main(int argc, char **argv)
         case HVM_SAVE_CODE(PIT): dump_pit(); break;
         case HVM_SAVE_CODE(RTC): dump_rtc(); break;
         case HVM_SAVE_CODE(HPET): dump_hpet(); break;
-        case HVM_SAVE_CODE(PMTIMER): dump_pmtimer(); break;
+        case HVM_SAVE_CODE(ACPI): dump_pmtimer(); break;
         case HVM_SAVE_CODE(MTRR): dump_mtrr(); break;
         case HVM_SAVE_CODE(VIRIDIAN_DOMAIN): dump_viridian_domain(); break;
         case HVM_SAVE_CODE(VIRIDIAN_VCPU): dump_viridian_vcpu(); break;
diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
index 99d1e86..5144928 100644
--- a/xen/arch/x86/hvm/pmtimer.c
+++ b/xen/arch/x86/hvm/pmtimer.c
@@ -56,9 +56,11 @@
 /* Dispatch SCIs based on the PM1a_STS and PM1a_EN registers */
 static void pmt_update_sci(PMTState *s)
 {
+    struct hvm_hw_acpi *acpi = &s->vcpu->domain->arch.hvm_domain.acpi;
+
     ASSERT(spin_is_locked(&s->lock));
 
-    if ( s->pm.pm1a_en & s->pm.pm1a_sts & SCI_MASK )
+    if ( acpi->pm1a_en & acpi->pm1a_sts & SCI_MASK )
         hvm_isa_irq_assert(s->vcpu->domain, SCI_IRQ);
     else
         hvm_isa_irq_deassert(s->vcpu->domain, SCI_IRQ);
@@ -72,7 +74,7 @@ void hvm_acpi_power_button(struct domain *d)
         return;
 
     spin_lock(&s->lock);
-    s->pm.pm1a_sts |= PWRBTN_STS;
+    d->arch.hvm_domain.acpi.pm1a_sts |= PWRBTN_STS;
     pmt_update_sci(s);
     spin_unlock(&s->lock);
 }
@@ -85,7 +87,7 @@ void hvm_acpi_sleep_button(struct domain *d)
         return;
 
     spin_lock(&s->lock);
-    s->pm.pm1a_sts |= SLPBTN_STS;
+    d->arch.hvm_domain.acpi.pm1a_sts |= PWRBTN_STS;
     pmt_update_sci(s);
     spin_unlock(&s->lock);
 }
@@ -95,7 +97,8 @@ void hvm_acpi_sleep_button(struct domain *d)
 static void pmt_update_time(PMTState *s)
 {
     uint64_t curr_gtime, tmp;
-    uint32_t tmr_val = s->pm.tmr_val, msb = tmr_val & TMR_VAL_MSB;
+    struct hvm_hw_acpi *acpi = &s->vcpu->domain->arch.hvm_domain.acpi;
+    uint32_t tmr_val = acpi->tmr_val, msb = tmr_val & TMR_VAL_MSB;
     
     ASSERT(spin_is_locked(&s->lock));
 
@@ -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;
 
     /* If the counter's MSB has changed, set the status bit */
     if ( (tmr_val & TMR_VAL_MSB) != msb )
     {
-        s->pm.pm1a_sts |= TMR_STS;
+        acpi->pm1a_sts |= TMR_STS;
         pmt_update_sci(s);
     }
 }
@@ -133,7 +136,8 @@ static void pmt_timer_callback(void *opaque)
     pmt_update_time(s);
 
     /* How close are we to the next MSB flip? */
-    pmt_cycles_until_flip = TMR_VAL_MSB - (s->pm.tmr_val & (TMR_VAL_MSB - 1));
+    pmt_cycles_until_flip = TMR_VAL_MSB -
+        (s->vcpu->domain->arch.hvm_domain.acpi.tmr_val & (TMR_VAL_MSB - 1));
 
     /* Overall time between MSB flips */
     time_until_flip = (1000000000ULL << 23) / FREQUENCE_PMTIMER;
@@ -152,6 +156,7 @@ static int handle_evt_io(
     int dir, unsigned int port, unsigned int bytes, uint32_t *val)
 {
     struct vcpu *v = current;
+    struct hvm_hw_acpi *acpi = &v->domain->arch.hvm_domain.acpi;
     PMTState *s = &v->domain->arch.hvm_domain.pl_time->vpmt;
     uint32_t addr, data, byte;
     int i;
@@ -175,16 +180,16 @@ static int handle_evt_io(
             {
                 /* PM1a_STS register bits are write-to-clear */
             case 0 /* PM1a_STS_ADDR */:
-                s->pm.pm1a_sts &= ~byte;
+                acpi->pm1a_sts &= ~byte;
                 break;
             case 1 /* PM1a_STS_ADDR + 1 */:
-                s->pm.pm1a_sts &= ~(byte << 8);
+                acpi->pm1a_sts &= ~(byte << 8);
                 break;
             case 2 /* PM1a_EN_ADDR */:
-                s->pm.pm1a_en = (s->pm.pm1a_en & 0xff00) | byte;
+                acpi->pm1a_en = (acpi->pm1a_en & 0xff00) | byte;
                 break;
             case 3 /* PM1a_EN_ADDR + 1 */:
-                s->pm.pm1a_en = (s->pm.pm1a_en & 0xff) | (byte << 8);
+                acpi->pm1a_en = (acpi->pm1a_en & 0xff) | (byte << 8);
                 break;
             default:
                 gdprintk(XENLOG_WARNING, 
@@ -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);
         data >>= 8 * addr;
         if ( bytes == 1 ) data &= 0xff;
         else if ( bytes == 2 ) data &= 0xffff;
@@ -215,6 +220,7 @@ static int handle_pmt_io(
     int dir, unsigned int port, unsigned int bytes, uint32_t *val)
 {
     struct vcpu *v = current;
+    struct hvm_hw_acpi *acpi = &v->domain->arch.hvm_domain.acpi;
     PMTState *s = &v->domain->arch.hvm_domain.pl_time->vpmt;
 
     if ( bytes != 4 || dir != IOREQ_READ )
@@ -226,7 +232,7 @@ static int handle_pmt_io(
     {
         /* We hold the lock: update timer value and return it. */
         pmt_update_time(s);
-        *val = s->pm.tmr_val;
+        *val = acpi->tmr_val;
         spin_unlock(&s->lock);
     }
     else
@@ -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));
     }
 
     return X86EMUL_OKAY;
 }
 
-static int pmtimer_save(struct domain *d, hvm_domain_context_t *h)
+static int acpi_save(struct domain *d, hvm_domain_context_t *h)
 {
+    struct hvm_hw_acpi *acpi = &d->arch.hvm_domain.acpi;
     PMTState *s = &d->arch.hvm_domain.pl_time->vpmt;
-    uint32_t x, msb = s->pm.tmr_val & TMR_VAL_MSB;
+    uint32_t x, msb = acpi->tmr_val & TMR_VAL_MSB;
     int rc;
 
     if ( !has_vpm(d) )
@@ -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;
+    if ( (acpi->tmr_val & TMR_VAL_MSB) != msb )
+        acpi->pm1a_sts |= TMR_STS;
     /* No point in setting the SCI here because we'll already have saved the 
      * IRQ and *PIC state; we'll fix it up when we restore the domain */
-
-    rc = hvm_save_entry(PMTIMER, 0, h, &s->pm);
+    rc = hvm_save_entry(ACPI, 0, h, acpi);
 
     spin_unlock(&s->lock);
 
     return rc;
 }
 
-static int pmtimer_load(struct domain *d, hvm_domain_context_t *h)
+static int acpi_load(struct domain *d, hvm_domain_context_t *h)
 {
+    struct hvm_hw_acpi *acpi = &d->arch.hvm_domain.acpi;
     PMTState *s = &d->arch.hvm_domain.pl_time->vpmt;
 
     if ( !has_vpm(d) )
@@ -284,7 +291,7 @@ static int pmtimer_load(struct domain *d, hvm_domain_context_t *h)
     spin_lock(&s->lock);
 
     /* Reload the registers */
-    if ( hvm_load_entry(PMTIMER, h, &s->pm) )
+    if ( hvm_load_entry(ACPI, h, acpi) )
     {
         spin_unlock(&s->lock);
         return -EINVAL;
@@ -302,7 +309,7 @@ static int pmtimer_load(struct domain *d, hvm_domain_context_t *h)
     return 0;
 }
 
-HVM_REGISTER_SAVE_RESTORE(PMTIMER, pmtimer_save, pmtimer_load, 
+HVM_REGISTER_SAVE_RESTORE(ACPI, acpi_save, acpi_load,
                           1, HVMSR_PER_DOM);
 
 int pmtimer_change_ioport(struct domain *d, unsigned int version)
@@ -377,5 +384,15 @@ void pmtimer_reset(struct domain *d)
         return;
 
     /* Reset the counter. */
-    d->arch.hvm_domain.pl_time->vpmt.pm.tmr_val = 0;
+    d->arch.hvm_domain.acpi.tmr_val = 0;
 }
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index f34d784..d55b432 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -102,6 +102,8 @@ struct hvm_domain {
     struct hvm_vioapic    *vioapic;
     struct hvm_hw_stdvga   stdvga;
 
+    struct hvm_hw_acpi     acpi;
+
     /* VCPU which is current target for 8259 interrupts. */
     struct vcpu           *i8259_target;
 
diff --git a/xen/include/asm-x86/hvm/vpt.h b/xen/include/asm-x86/hvm/vpt.h
index a27bea4..1b7213d 100644
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -121,7 +121,6 @@ typedef struct RTCState {
 
 #define FREQUENCE_PMTIMER  3579545  /* Timer should run at 3.579545 MHz */
 typedef struct PMTState {
-    struct hvm_hw_pmtimer pm;   /* 32bit timer value */
     struct vcpu *vcpu;          /* Keeps sync with this vcpu's guest-time */
     uint64_t last_gtime;        /* Last (guest) time we updated the timer */
     uint32_t not_accounted;     /* time not accounted at last update */
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 8d73b51..3997487 100644
--- 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);
 
 /*
  * MTRR MSRs
-- 
2.7.4


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

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

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> 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

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

[Boris Ostrovsky]

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: [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

[Andrew Cooper]

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: [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

[Boris Ostrovsky]

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: [PATCH v4 01/15] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain

[Wei Liu]

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 <boris.ostrovsky@oracle.com>
> ---
> Changes in v4:
> * New patch
> 
>  tools/misc/xen-hvmctx.c                |  4 +-

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

[PATCH v4 02/15] acpi: Make pmtimer optional in FADT

[Boris Ostrovsky]

PM timer is not supported by PVH guests.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 tools/firmware/hvmloader/util.c | 3 ++-
 tools/libacpi/build.c           | 5 +++++
 tools/libacpi/libacpi.h         | 1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 6e0cfe7..1d78973 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -948,7 +948,8 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
     if ( !strncmp(xenstore_read("platform/acpi_s4", "1"), "1", 1)  )
         config->table_flags |= ACPI_HAS_SSDT_S4;
 
-    config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC | ACPI_HAS_WAET);
+    config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC |
+                            ACPI_HAS_WAET | ACPI_HAS_PMTIMER);
 
     config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
 
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index 47dae01..e1fd381 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -574,6 +574,11 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
 
     fadt = ctxt->mem_ops.alloc(ctxt, sizeof(struct acpi_20_fadt), 16);
     if (!fadt) goto oom;
+    if ( !(config->table_flags & ACPI_HAS_PMTIMER) )
+    {
+        Fadt.pm_tmr_blk = Fadt.pm_tmr_len = 0;
+        memset(&Fadt.x_pm_tmr_blk, 0, sizeof(Fadt.x_pm_tmr_blk));
+    }
     memcpy(fadt, &Fadt, sizeof(struct acpi_20_fadt));
     fadt->dsdt   = ctxt->mem_ops.v2p(ctxt, dsdt);
     fadt->x_dsdt = ctxt->mem_ops.v2p(ctxt, dsdt);
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index 1d388f9..bda692e 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -30,6 +30,7 @@
 #define ACPI_HAS_TCPA        (1<<7)
 #define ACPI_HAS_IOAPIC      (1<<8)
 #define ACPI_HAS_WAET        (1<<9)
+#define ACPI_HAS_PMTIMER     (1<<10)
 
 struct xen_vmemrange;
 struct acpi_numa {
-- 
2.7.4


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

[PATCH v4 03/15] acpi: Power and Sleep ACPI buttons are not emulated for PVH guests

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 tools/firmware/hvmloader/util.c | 3 ++-
 tools/libacpi/build.c           | 2 ++
 tools/libacpi/libacpi.h         | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
index 1d78973..a3f12fe 100644
--- a/tools/firmware/hvmloader/util.c
+++ b/tools/firmware/hvmloader/util.c
@@ -949,7 +949,8 @@ void hvmloader_acpi_build_tables(struct acpi_config *config,
         config->table_flags |= ACPI_HAS_SSDT_S4;
 
     config->table_flags |= (ACPI_HAS_TCPA | ACPI_HAS_IOAPIC |
-                            ACPI_HAS_WAET | ACPI_HAS_PMTIMER);
+                            ACPI_HAS_WAET | ACPI_HAS_PMTIMER |
+                            ACPI_HAS_BUTTONS);
 
     config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
 
diff --git a/tools/libacpi/build.c b/tools/libacpi/build.c
index e1fd381..4a2e2a9 100644
--- a/tools/libacpi/build.c
+++ b/tools/libacpi/build.c
@@ -579,6 +579,8 @@ int acpi_build_tables(struct acpi_ctxt *ctxt, struct acpi_config *config)
         Fadt.pm_tmr_blk = Fadt.pm_tmr_len = 0;
         memset(&Fadt.x_pm_tmr_blk, 0, sizeof(Fadt.x_pm_tmr_blk));
     }
+    if ( !(config->table_flags & ACPI_HAS_BUTTONS) )
+        Fadt.flags |= (ACPI_PWR_BUTTON | ACPI_SLP_BUTTON);
     memcpy(fadt, &Fadt, sizeof(struct acpi_20_fadt));
     fadt->dsdt   = ctxt->mem_ops.v2p(ctxt, dsdt);
     fadt->x_dsdt = ctxt->mem_ops.v2p(ctxt, dsdt);
diff --git a/tools/libacpi/libacpi.h b/tools/libacpi/libacpi.h
index bda692e..dd6ef8b 100644
--- a/tools/libacpi/libacpi.h
+++ b/tools/libacpi/libacpi.h
@@ -31,6 +31,7 @@
 #define ACPI_HAS_IOAPIC      (1<<8)
 #define ACPI_HAS_WAET        (1<<9)
 #define ACPI_HAS_PMTIMER     (1<<10)
+#define ACPI_HAS_BUTTONS     (1<<11)
 
 struct xen_vmemrange;
 struct acpi_numa {
-- 
2.7.4


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

[PATCH v4 04/15] acpi: PVH guests need _E02 method

[Boris Ostrovsky]

This is the method that will get invoked on an SCI.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 tools/libacpi/mk_dsdt.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
index 16320a9..5765822 100644
--- a/tools/libacpi/mk_dsdt.c
+++ b/tools/libacpi/mk_dsdt.c
@@ -280,11 +280,6 @@ int main(int argc, char **argv)
 
     pop_block();
 
-    if (dm_version == QEMU_NONE) {
-        pop_block();
-        return 0;
-    }
-
     /* Define GPE control method. */
     push_block("Scope", "\\_GPE");
     push_block("Method",
@@ -292,6 +287,11 @@ int main(int argc, char **argv)
     stmt("\\_SB.PRSC ()", NULL);
     pop_block();
     pop_block();
+
+    if (dm_version == QEMU_NONE) {
+        pop_block();
+        return 0;
+    }
     /**** Processor end ****/
 
 
-- 
2.7.4


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

[PATCH v4 05/15] acpi/x86: Define ACPI IO registers for PVH guests

[Boris Ostrovsky]

Define VCPU available map address (used by AML's PRSC method)
and GPE0 CPU hotplug event number. Use these definitions in mk_dsdt
instead hardcoded values.

These definitions will later be used by both the hypervisor and
the toolstack (initially for PVH guests only), thus they are
placed in public headers.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
* Macros for registers length and offset stay in static_tables.c,
  a comment statig that they should not change without corresponding
  change to struct hvm_hw_acpi is added
* Definitions of EN_ACPI_CPU_MAP* and  XEN_GPE0_CPUHP_BIT are
  moved to include/public/arch-x86/xen.h

 tools/libacpi/mk_dsdt.c           | 7 +++++--
 tools/libacpi/static_tables.c     | 4 ++++
 xen/include/public/arch-x86/xen.h | 7 +++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
index 5765822..606e7a0 100644
--- a/tools/libacpi/mk_dsdt.c
+++ b/tools/libacpi/mk_dsdt.c
@@ -18,6 +18,7 @@
 #include <stdlib.h>
 #include <stdbool.h>
 #if defined(__i386__) || defined(__x86_64__)
+#include <xen/arch-x86/xen.h>
 #include <xen/hvm/hvm_info_table.h>
 #elif defined(__aarch64__)
 #include <xen/arch-arm.h>
@@ -244,7 +245,8 @@ int main(int argc, char **argv)
 #endif
 
     /* Operation Region 'PRST': bitmask of online CPUs. */
-    stmt("OperationRegion", "PRST, SystemIO, 0xaf00, 32");
+    stmt("OperationRegion", "PRST, SystemIO, %#x, %d",
+        XEN_ACPI_CPU_MAP, XEN_ACPI_CPU_MAP_LEN);
     push_block("Field", "PRST, ByteAcc, NoLock, Preserve");
     indent(); printf("PRS, %u\n", max_cpus);
     pop_block();
@@ -283,7 +285,8 @@ int main(int argc, char **argv)
     /* Define GPE control method. */
     push_block("Scope", "\\_GPE");
     push_block("Method",
-               dm_version == QEMU_XEN_TRADITIONAL ? "_L02" : "_E02");
+               dm_version == QEMU_XEN_TRADITIONAL ? "_L%02d" : "_E%02d",
+               XEN_GPE0_CPUHP_BIT);
     stmt("\\_SB.PRSC ()", NULL);
     pop_block();
     pop_block();
diff --git a/tools/libacpi/static_tables.c b/tools/libacpi/static_tables.c
index 617bf68..f48b954 100644
--- a/tools/libacpi/static_tables.c
+++ b/tools/libacpi/static_tables.c
@@ -31,6 +31,10 @@ struct acpi_20_facs Facs = {
  * Fixed ACPI Description Table (FADT).
  */
 
+/*
+ * These values must match register definitions in struct hvm_hw_acpi
+ * (in xen/include/public/arch-x86/hvm/save.h).
+ */
 #define ACPI_PM1A_EVT_BLK_BIT_WIDTH         0x20
 #define ACPI_PM1A_EVT_BLK_BIT_OFFSET        0x00
 #define ACPI_PM1A_CNT_BLK_BIT_WIDTH         0x10
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index cdd93c1..0e3a3df 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -291,6 +291,13 @@ struct xen_arch_domainconfig {
                                      XEN_X86_EMU_PIT)
     uint32_t emulation_flags;
 };
+
+/* Location of online VCPU bitmap. */
+#define XEN_ACPI_CPU_MAP             0xaf00
+#define XEN_ACPI_CPU_MAP_LEN         ((HVM_MAX_VCPUS + 7) / 8)
+
+/* GPE0 bit set during CPU hotplug */
+#define XEN_GPE0_CPUHP_BIT           2
 #endif
 
 #endif /* !__ASSEMBLY__ */
-- 
2.7.4


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

Re: [PATCH v4 05/15] acpi/x86: Define ACPI IO registers for PVH guests

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -291,6 +291,13 @@ struct xen_arch_domainconfig {
>                                       XEN_X86_EMU_PIT)
>      uint32_t emulation_flags;
>  };
> +
> +/* Location of online VCPU bitmap. */
> +#define XEN_ACPI_CPU_MAP             0xaf00
> +#define XEN_ACPI_CPU_MAP_LEN         ((HVM_MAX_VCPUS + 7) / 8)
> +
> +/* GPE0 bit set during CPU hotplug */
> +#define XEN_GPE0_CPUHP_BIT           2

Here I'm unsure - isn't this an ACPI specific register? If so, the name
would better be XEN_ACPI_GPE0_CPUHP_BIT.

With that (or with a good reason why the current name is better)
Reviewed-by: Jan Beulich <jbeulich@suse.com>
but the patch needs re-basing afaict.

Jan


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

Re: [PATCH v4 05/15] acpi/x86: Define ACPI IO registers for PVH guests

[Boris Ostrovsky]

On 12/01/2016 10:57 AM, Jan Beulich wrote:
>>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/include/public/arch-x86/xen.h
>> +++ b/xen/include/public/arch-x86/xen.h
>> @@ -291,6 +291,13 @@ struct xen_arch_domainconfig {
>>                                       XEN_X86_EMU_PIT)
>>      uint32_t emulation_flags;
>>  };
>> +
>> +/* Location of online VCPU bitmap. */
>> +#define XEN_ACPI_CPU_MAP             0xaf00
>> +#define XEN_ACPI_CPU_MAP_LEN         ((HVM_MAX_VCPUS + 7) / 8)
>> +
>> +/* GPE0 bit set during CPU hotplug */
>> +#define XEN_GPE0_CPUHP_BIT           2
> Here I'm unsure - isn't this an ACPI specific register? If so, the name
> would better be XEN_ACPI_GPE0_CPUHP_BIT.
>
> With that (or with a good reason why the current name is better)

No, it should have "ACPI".

-boris


> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> but the patch needs re-basing afaict.
>
> Jan
>


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

[PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Boris Ostrovsky]

This domctl will allow toolstack to read and write some
ACPI registers. It will be available to both x86 and ARM
but will be implemented first only for x86

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
* New patch

 xen/arch/x86/domctl.c            |  9 +++++++++
 xen/arch/x86/hvm/Makefile        |  1 +
 xen/arch/x86/hvm/acpi.c          | 24 ++++++++++++++++++++++++
 xen/include/asm-x86/hvm/domain.h |  3 +++
 xen/include/public/domctl.h      | 25 +++++++++++++++++++++++++
 5 files changed, 62 insertions(+)
 create mode 100644 xen/arch/x86/hvm/acpi.c

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 2a2fe04..111bcbb 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1430,6 +1430,15 @@ long arch_do_domctl(
         }
         break;
 
+    case XEN_DOMCTL_acpi_access:
+        if ( !is_hvm_domain(d) )
+            ret = -EINVAL;
+        else
+            ret = hvm_acpi_domctl_access(d, domctl->u.acpi_access.rw,
+                                         &domctl->u.acpi_access.gas,
+                                         domctl->u.acpi_access.val);
+        break;
+
     default:
         ret = iommu_do_domctl(domctl, d, u_domctl);
         break;
diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
index f750d13..bae3244 100644
--- a/xen/arch/x86/hvm/Makefile
+++ b/xen/arch/x86/hvm/Makefile
@@ -1,6 +1,7 @@
 subdir-y += svm
 subdir-y += vmx
 
+obj-y += acpi.o
 obj-y += asid.o
 obj-y += emulate.o
 obj-y += hpet.o
diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
new file mode 100644
index 0000000..7d42eaf
--- /dev/null
+++ b/xen/arch/x86/hvm/acpi.c
@@ -0,0 +1,24 @@
+/* acpi.c: ACPI access handling
+ *
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+#include <xen/errno.h>
+#include <xen/lib.h>
+#include <xen/sched.h>
+
+
+int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
+                           XEN_GUEST_HANDLE_PARAM(uint8) arg)
+{
+    return -ENOSYS;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index d55b432..c5cd86c 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -158,6 +158,9 @@ struct hvm_domain {
 
 #define hap_enabled(d)  ((d)->arch.hvm_domain.hap_enabled)
 
+int hvm_acpi_domctl_access(struct domain *currd, uint8_t rw, gas_t *gas,
+                           XEN_GUEST_HANDLE_PARAM(uint8) arg);
+
 #endif /* __ASM_X86_HVM_DOMAIN_H__ */
 
 /*
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 177319d..26fe009 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
 typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
 
+/* ACPI Generic Address Structure */
+typedef struct gas {
+#define XEN_ACPI_SYSTEM_MEMORY 0
+#define XEN_ACPI_SYSTEM_IO     1
+    uint8_t    space_id;           /* Address space */
+    uint8_t    bit_width;          /* Size in bits of given register */
+    uint8_t    bit_offset;         /* Bit offset within the register */
+    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
+    uint64_t   address;            /* 64-bit address of register */
+} gas_t;
+
+struct xen_domctl_acpi_access {
+    gas_t      gas;                    /* IN: Register being accessed */
+
+#define XEN_DOMCTL_ACPI_READ   0
+#define XEN_DOMCTL_ACPI_WRITE  1
+    uint8_t    rw;                     /* IN: Read or write */
+
+    XEN_GUEST_HANDLE_64(uint8) val;    /* IN/OUT: data */
+};
+typedef struct xen_domctl_acpi_access xen_domctl_acpi_access_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_acpi_access_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -1221,6 +1244,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_monitor_op                    77
 #define XEN_DOMCTL_psr_cat_op                    78
 #define XEN_DOMCTL_soft_reset                    79
+#define XEN_DOMCTL_acpi_access                   80
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -1283,6 +1307,7 @@ struct xen_domctl {
         struct xen_domctl_psr_cmt_op        psr_cmt_op;
         struct xen_domctl_monitor_op        monitor_op;
         struct xen_domctl_psr_cat_op        psr_cat_op;
+        struct xen_domctl_acpi_access       acpi_access;
         uint8_t                             pad[128];
     } u;
 };
-- 
2.7.4


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
> --- /dev/null
> +++ b/xen/arch/x86/hvm/acpi.c
> @@ -0,0 +1,24 @@
> +/* acpi.c: ACPI access handling
> + *
> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> + */
> +#include <xen/errno.h>
> +#include <xen/lib.h>
> +#include <xen/sched.h>
> +
> +
> +int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,

I guess the gas pointer can be const?

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>  
> +/* ACPI Generic Address Structure */
> +typedef struct gas {

xen_acpi_gas

> +#define XEN_ACPI_SYSTEM_MEMORY 0
> +#define XEN_ACPI_SYSTEM_IO     1
> +    uint8_t    space_id;           /* Address space */
> +    uint8_t    bit_width;          /* Size in bits of given register */
> +    uint8_t    bit_offset;         /* Bit offset within the register */
> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
> +    uint64_t   address;            /* 64-bit address of register */

uint64_aligned_t with explicit padding added ahead of it.

And then there's the question of what uses of this will look like:
I'm not convinced we need to stick to the exact ACPI layout
here, unless you expect (or could imagine) for the tool stack to
hold GAS structures coming from elsewhere in its hands. If we
don't follow the layout as strictly, we could namely widen
bit_width (and maybe bit_offset) to allow for larger transfers
in one go. And in such a relaxed model I don't think we'd need
access_width at all as a field.

> +} gas_t;

xen_acpi_gas_t

> +
> +struct xen_domctl_acpi_access {
> +    gas_t      gas;                    /* IN: Register being accessed */
> +
> +#define XEN_DOMCTL_ACPI_READ   0
> +#define XEN_DOMCTL_ACPI_WRITE  1
> +    uint8_t    rw;                     /* IN: Read or write */
> +
> +    XEN_GUEST_HANDLE_64(uint8) val;    /* IN/OUT: data */

I'd prefer if we made this void, as there's no type associated with
the data really. And please add explicit padding again.

Jan


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Boris Ostrovsky]

On 12/01/2016 11:06 AM, Jan Beulich wrote:
>
>> +++ b/xen/include/public/domctl.h
>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>  
>> +/* ACPI Generic Address Structure */
>> +typedef struct gas {
> xen_acpi_gas
>
>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>> +#define XEN_ACPI_SYSTEM_IO     1
>> +    uint8_t    space_id;           /* Address space */
>> +    uint8_t    bit_width;          /* Size in bits of given register */
>> +    uint8_t    bit_offset;         /* Bit offset within the register */
>> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
>> +    uint64_t   address;            /* 64-bit address of register */
> uint64_aligned_t with explicit padding added ahead of it.
>
> And then there's the question of what uses of this will look like:
> I'm not convinced we need to stick to the exact ACPI layout
> here, unless you expect (or could imagine) for the tool stack to
> hold GAS structures coming from elsewhere in its hands. If we
> don't follow the layout as strictly, we could namely widen
> bit_width (and maybe bit_offset) to allow for larger transfers
> in one go. And in such a relaxed model I don't think we'd need
> access_width at all as a field.

There is indeed no current need to use actual ACPI GAS layout but then
it's not GAS, really, and should be named something else.

-boris


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Jan Beulich]

>>> On 01.12.16 at 17:43, <boris.ostrovsky@oracle.com> wrote:
> On 12/01/2016 11:06 AM, Jan Beulich wrote:
>>
>>> +++ b/xen/include/public/domctl.h
>>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>>  
>>> +/* ACPI Generic Address Structure */
>>> +typedef struct gas {
>> xen_acpi_gas
>>
>>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>>> +#define XEN_ACPI_SYSTEM_IO     1
>>> +    uint8_t    space_id;           /* Address space */
>>> +    uint8_t    bit_width;          /* Size in bits of given register */
>>> +    uint8_t    bit_offset;         /* Bit offset within the register */
>>> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
>>> +    uint64_t   address;            /* 64-bit address of register */
>> uint64_aligned_t with explicit padding added ahead of it.
>>
>> And then there's the question of what uses of this will look like:
>> I'm not convinced we need to stick to the exact ACPI layout
>> here, unless you expect (or could imagine) for the tool stack to
>> hold GAS structures coming from elsewhere in its hands. If we
>> don't follow the layout as strictly, we could namely widen
>> bit_width (and maybe bit_offset) to allow for larger transfers
>> in one go. And in such a relaxed model I don't think we'd need
>> access_width at all as a field.
> 
> There is indeed no current need to use actual ACPI GAS layout but then
> it's not GAS, really, and should be named something else.

Which of course is fine by me; I had referred to that structure only
for the underlying principle of specifying how to access the data.

Jan


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Boris Ostrovsky]

On 12/02/2016 02:48 AM, Jan Beulich wrote:
>>>> On 01.12.16 at 17:43, <boris.ostrovsky@oracle.com> wrote:
>> On 12/01/2016 11:06 AM, Jan Beulich wrote:
>>>
>>>> +++ b/xen/include/public/domctl.h
>>>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>>>
>>>> +/* ACPI Generic Address Structure */
>>>> +typedef struct gas {
>>> xen_acpi_gas
>>>
>>>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>>>> +#define XEN_ACPI_SYSTEM_IO     1
>>>> +    uint8_t    space_id;           /* Address space */
>>>> +    uint8_t    bit_width;          /* Size in bits of given register */
>>>> +    uint8_t    bit_offset;         /* Bit offset within the register */
>>>> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
>>>> +    uint64_t   address;            /* 64-bit address of register */
>>> uint64_aligned_t with explicit padding added ahead of it.
>>>
>>> And then there's the question of what uses of this will look like:
>>> I'm not convinced we need to stick to the exact ACPI layout
>>> here, unless you expect (or could imagine) for the tool stack to
>>> hold GAS structures coming from elsewhere in its hands. If we
>>> don't follow the layout as strictly, we could namely widen
>>> bit_width (and maybe bit_offset) to allow for larger transfers
>>> in one go. And in such a relaxed model I don't think we'd need
>>> access_width at all as a field.
>>
>> There is indeed no current need to use actual ACPI GAS layout but then
>> it's not GAS, really, and should be named something else.
>
> Which of course is fine by me; I had referred to that structure only
> for the underlying principle of specifying how to access the data.

Are there any registers that are not byte-aligned or not whole number of 
bytes?

I am thinking about dropping bit_offset (along with access_width) and 
making bit_width (byte_)width. And keeping the latter as uint8_t will 
also implicitly limit register size to 256 bytes which I think is a 
reasonable size limit.

-boris

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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Jan Beulich]

>>> On 12.12.16 at 14:08, <boris.ostrovsky@oracle.com> wrote:

> 
> On 12/02/2016 02:48 AM, Jan Beulich wrote:
>>>>> On 01.12.16 at 17:43, <boris.ostrovsky@oracle.com> wrote:
>>> On 12/01/2016 11:06 AM, Jan Beulich wrote:
>>>>
>>>>> +++ b/xen/include/public/domctl.h
>>>>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>>>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>>>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>>>>
>>>>> +/* ACPI Generic Address Structure */
>>>>> +typedef struct gas {
>>>> xen_acpi_gas
>>>>
>>>>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>>>>> +#define XEN_ACPI_SYSTEM_IO     1
>>>>> +    uint8_t    space_id;           /* Address space */
>>>>> +    uint8_t    bit_width;          /* Size in bits of given register */
>>>>> +    uint8_t    bit_offset;         /* Bit offset within the register */
>>>>> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
>>>>> +    uint64_t   address;            /* 64-bit address of register */
>>>> uint64_aligned_t with explicit padding added ahead of it.
>>>>
>>>> And then there's the question of what uses of this will look like:
>>>> I'm not convinced we need to stick to the exact ACPI layout
>>>> here, unless you expect (or could imagine) for the tool stack to
>>>> hold GAS structures coming from elsewhere in its hands. If we
>>>> don't follow the layout as strictly, we could namely widen
>>>> bit_width (and maybe bit_offset) to allow for larger transfers
>>>> in one go. And in such a relaxed model I don't think we'd need
>>>> access_width at all as a field.
>>>
>>> There is indeed no current need to use actual ACPI GAS layout but then
>>> it's not GAS, really, and should be named something else.
>>
>> Which of course is fine by me; I had referred to that structure only
>> for the underlying principle of specifying how to access the data.
> 
> Are there any registers that are not byte-aligned or not whole number of 
> bytes?
> 
> I am thinking about dropping bit_offset (along with access_width) and 
> making bit_width (byte_)width. And keeping the latter as uint8_t will 
> also implicitly limit register size to 256 bytes which I think is a 
> reasonable size limit.

Since we're doing the emulation (and hence defining the registers)
we could require no such unusual registers. This would be something
we can't simplify only if we foresee ever needing to hand through a
hardware register without interposing any emulation.

Whether limiting to 256 bytes is reasonable I'm not so sure, otoh.

Jan


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Boris Ostrovsky]

On 12/12/2016 09:02 AM, Jan Beulich wrote:
>>>> On 12.12.16 at 14:08, <boris.ostrovsky@oracle.com> wrote:
>> On 12/02/2016 02:48 AM, Jan Beulich wrote:
>>>>>> On 01.12.16 at 17:43, <boris.ostrovsky@oracle.com> wrote:
>>>> On 12/01/2016 11:06 AM, Jan Beulich wrote:
>>>>>> +++ b/xen/include/public/domctl.h
>>>>>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>>>>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>>>>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>>>>>
>>>>>> +/* ACPI Generic Address Structure */
>>>>>> +typedef struct gas {
>>>>> xen_acpi_gas
>>>>>
>>>>>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>>>>>> +#define XEN_ACPI_SYSTEM_IO     1
>>>>>> +    uint8_t    space_id;           /* Address space */
>>>>>> +    uint8_t    bit_width;          /* Size in bits of given register */
>>>>>> +    uint8_t    bit_offset;         /* Bit offset within the register */
>>>>>> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
>>>>>> +    uint64_t   address;            /* 64-bit address of register */
>>>>> uint64_aligned_t with explicit padding added ahead of it.
>>>>>
>>>>> And then there's the question of what uses of this will look like:
>>>>> I'm not convinced we need to stick to the exact ACPI layout
>>>>> here, unless you expect (or could imagine) for the tool stack to
>>>>> hold GAS structures coming from elsewhere in its hands. If we
>>>>> don't follow the layout as strictly, we could namely widen
>>>>> bit_width (and maybe bit_offset) to allow for larger transfers
>>>>> in one go. And in such a relaxed model I don't think we'd need
>>>>> access_width at all as a field.
>>>> There is indeed no current need to use actual ACPI GAS layout but then
>>>> it's not GAS, really, and should be named something else.
>>> Which of course is fine by me; I had referred to that structure only
>>> for the underlying principle of specifying how to access the data.
>> Are there any registers that are not byte-aligned or not whole number of 
>> bytes?
>>
>> I am thinking about dropping bit_offset (along with access_width) and 
>> making bit_width (byte_)width. And keeping the latter as uint8_t will 
>> also implicitly limit register size to 256 bytes which I think is a 
>> reasonable size limit.
> Since we're doing the emulation (and hence defining the registers)
> we could require no such unusual registers. This would be something
> we can't simplify only if we foresee ever needing to hand through a
> hardware register without interposing any emulation.
>
> Whether limiting to 256 bytes is reasonable I'm not so sure, otoh.

When would we ever need to access anything larger? I'd think that the
common case is a few (1-4) bytes. The one instance when this is not true
is the VCPU map and 256 bytes allow for 16K VCPUs, which I suspect we
won't reach in a while.

But I can increase the length to uint16_t if you feel it's would be better.

-boris


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Jan Beulich]

>>> On 12.12.16 at 17:19, <boris.ostrovsky@oracle.com> wrote:
> On 12/12/2016 09:02 AM, Jan Beulich wrote:
>>>>> On 12.12.16 at 14:08, <boris.ostrovsky@oracle.com> wrote:
>>> On 12/02/2016 02:48 AM, Jan Beulich wrote:
>>>>>>> On 01.12.16 at 17:43, <boris.ostrovsky@oracle.com> wrote:
>>>>> On 12/01/2016 11:06 AM, Jan Beulich wrote:
>>>>>>> +++ b/xen/include/public/domctl.h
>>>>>>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>>>>>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>>>>>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>>>>>>
>>>>>>> +/* ACPI Generic Address Structure */
>>>>>>> +typedef struct gas {
>>>>>> xen_acpi_gas
>>>>>>
>>>>>>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>>>>>>> +#define XEN_ACPI_SYSTEM_IO     1
>>>>>>> +    uint8_t    space_id;           /* Address space */
>>>>>>> +    uint8_t    bit_width;          /* Size in bits of given register */
>>>>>>> +    uint8_t    bit_offset;         /* Bit offset within the register */
>>>>>>> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
>>>>>>> +    uint64_t   address;            /* 64-bit address of register */
>>>>>> uint64_aligned_t with explicit padding added ahead of it.
>>>>>>
>>>>>> And then there's the question of what uses of this will look like:
>>>>>> I'm not convinced we need to stick to the exact ACPI layout
>>>>>> here, unless you expect (or could imagine) for the tool stack to
>>>>>> hold GAS structures coming from elsewhere in its hands. If we
>>>>>> don't follow the layout as strictly, we could namely widen
>>>>>> bit_width (and maybe bit_offset) to allow for larger transfers
>>>>>> in one go. And in such a relaxed model I don't think we'd need
>>>>>> access_width at all as a field.
>>>>> There is indeed no current need to use actual ACPI GAS layout but then
>>>>> it's not GAS, really, and should be named something else.
>>>> Which of course is fine by me; I had referred to that structure only
>>>> for the underlying principle of specifying how to access the data.
>>> Are there any registers that are not byte-aligned or not whole number of 
>>> bytes?
>>>
>>> I am thinking about dropping bit_offset (along with access_width) and 
>>> making bit_width (byte_)width. And keeping the latter as uint8_t will 
>>> also implicitly limit register size to 256 bytes which I think is a 
>>> reasonable size limit.
>> Since we're doing the emulation (and hence defining the registers)
>> we could require no such unusual registers. This would be something
>> we can't simplify only if we foresee ever needing to hand through a
>> hardware register without interposing any emulation.
>>
>> Whether limiting to 256 bytes is reasonable I'm not so sure, otoh.
> 
> When would we ever need to access anything larger? I'd think that the
> common case is a few (1-4) bytes. The one instance when this is not true
> is the VCPU map and 256 bytes allow for 16K VCPUs, which I suspect we
> won't reach in a while.
> 
> But I can increase the length to uint16_t if you feel it's would be better.

It's domctl, so we can change it later anyway. As said - I'm not
really sure here.

Jan


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Julien Grall]

Hi Boris,

On 29/11/16 15:33, Boris Ostrovsky wrote:
> This domctl will allow toolstack to read and write some
> ACPI registers. It will be available to both x86 and ARM
> but will be implemented first only for x86

Can you explain why we would need this on ARM?

Cheers,

>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> Changes in v4:
> * New patch
>
>  xen/arch/x86/domctl.c            |  9 +++++++++
>  xen/arch/x86/hvm/Makefile        |  1 +
>  xen/arch/x86/hvm/acpi.c          | 24 ++++++++++++++++++++++++
>  xen/include/asm-x86/hvm/domain.h |  3 +++
>  xen/include/public/domctl.h      | 25 +++++++++++++++++++++++++
>  5 files changed, 62 insertions(+)
>  create mode 100644 xen/arch/x86/hvm/acpi.c
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 2a2fe04..111bcbb 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1430,6 +1430,15 @@ long arch_do_domctl(
>          }
>          break;
>
> +    case XEN_DOMCTL_acpi_access:
> +        if ( !is_hvm_domain(d) )
> +            ret = -EINVAL;
> +        else
> +            ret = hvm_acpi_domctl_access(d, domctl->u.acpi_access.rw,
> +                                         &domctl->u.acpi_access.gas,
> +                                         domctl->u.acpi_access.val);
> +        break;
> +
>      default:
>          ret = iommu_do_domctl(domctl, d, u_domctl);
>          break;
> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
> index f750d13..bae3244 100644
> --- a/xen/arch/x86/hvm/Makefile
> +++ b/xen/arch/x86/hvm/Makefile
> @@ -1,6 +1,7 @@
>  subdir-y += svm
>  subdir-y += vmx
>
> +obj-y += acpi.o
>  obj-y += asid.o
>  obj-y += emulate.o
>  obj-y += hpet.o
> diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
> new file mode 100644
> index 0000000..7d42eaf
> --- /dev/null
> +++ b/xen/arch/x86/hvm/acpi.c
> @@ -0,0 +1,24 @@
> +/* acpi.c: ACPI access handling
> + *
> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> + */
> +#include <xen/errno.h>
> +#include <xen/lib.h>
> +#include <xen/sched.h>
> +
> +
> +int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
> +                           XEN_GUEST_HANDLE_PARAM(uint8) arg)
> +{
> +    return -ENOSYS;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
> index d55b432..c5cd86c 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -158,6 +158,9 @@ struct hvm_domain {
>
>  #define hap_enabled(d)  ((d)->arch.hvm_domain.hap_enabled)
>
> +int hvm_acpi_domctl_access(struct domain *currd, uint8_t rw, gas_t *gas,
> +                           XEN_GUEST_HANDLE_PARAM(uint8) arg);
> +
>  #endif /* __ASM_X86_HVM_DOMAIN_H__ */
>
>  /*
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 177319d..26fe009 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>
> +/* ACPI Generic Address Structure */
> +typedef struct gas {
> +#define XEN_ACPI_SYSTEM_MEMORY 0
> +#define XEN_ACPI_SYSTEM_IO     1
> +    uint8_t    space_id;           /* Address space */
> +    uint8_t    bit_width;          /* Size in bits of given register */
> +    uint8_t    bit_offset;         /* Bit offset within the register */
> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
> +    uint64_t   address;            /* 64-bit address of register */
> +} gas_t;
> +
> +struct xen_domctl_acpi_access {
> +    gas_t      gas;                    /* IN: Register being accessed */
> +
> +#define XEN_DOMCTL_ACPI_READ   0
> +#define XEN_DOMCTL_ACPI_WRITE  1
> +    uint8_t    rw;                     /* IN: Read or write */
> +
> +    XEN_GUEST_HANDLE_64(uint8) val;    /* IN/OUT: data */
> +};
> +typedef struct xen_domctl_acpi_access xen_domctl_acpi_access_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_acpi_access_t);
> +
>  struct xen_domctl {
>      uint32_t cmd;
>  #define XEN_DOMCTL_createdomain                   1
> @@ -1221,6 +1244,7 @@ struct xen_domctl {
>  #define XEN_DOMCTL_monitor_op                    77
>  #define XEN_DOMCTL_psr_cat_op                    78
>  #define XEN_DOMCTL_soft_reset                    79
> +#define XEN_DOMCTL_acpi_access                   80
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -1283,6 +1307,7 @@ struct xen_domctl {
>          struct xen_domctl_psr_cmt_op        psr_cmt_op;
>          struct xen_domctl_monitor_op        monitor_op;
>          struct xen_domctl_psr_cat_op        psr_cat_op;
> +        struct xen_domctl_acpi_access       acpi_access;
>          uint8_t                             pad[128];
>      } u;
>  };
>

-- 
Julien Grall

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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Boris Ostrovsky]

On 12/12/2016 08:28 AM, Julien Grall wrote:
> Hi Boris,
>
> On 29/11/16 15:33, Boris Ostrovsky wrote:
>> This domctl will allow toolstack to read and write some
>> ACPI registers. It will be available to both x86 and ARM
>> but will be implemented first only for x86
>
> Can you explain why we would need this on ARM?

What this is meant to say is that the new domctl is not specific to x86
and therefore is available as a common (as opposed to arch-specific)
interface.

If ARM ever needs to access ACPI space from toostack it should be able
to use it (provided that an implementation for that is available in
arch/arm).

-boris


>
> Cheers,
>
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>> Changes in v4:
>> * New patch
>>
>>  xen/arch/x86/domctl.c            |  9 +++++++++
>>  xen/arch/x86/hvm/Makefile        |  1 +
>>  xen/arch/x86/hvm/acpi.c          | 24 ++++++++++++++++++++++++
>>  xen/include/asm-x86/hvm/domain.h |  3 +++
>>  xen/include/public/domctl.h      | 25 +++++++++++++++++++++++++
>>  5 files changed, 62 insertions(+)
>>  create mode 100644 xen/arch/x86/hvm/acpi.c
>>
>> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
>> index 2a2fe04..111bcbb 100644
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -1430,6 +1430,15 @@ long arch_do_domctl(
>>          }
>>          break;
>>
>> +    case XEN_DOMCTL_acpi_access:
>> +        if ( !is_hvm_domain(d) )
>> +            ret = -EINVAL;
>> +        else
>> +            ret = hvm_acpi_domctl_access(d, domctl->u.acpi_access.rw,
>> +                                         &domctl->u.acpi_access.gas,
>> +                                         domctl->u.acpi_access.val);
>> +        break;
>> +
>>      default:
>>          ret = iommu_do_domctl(domctl, d, u_domctl);
>>          break;
>> diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile
>> index f750d13..bae3244 100644
>> --- a/xen/arch/x86/hvm/Makefile
>> +++ b/xen/arch/x86/hvm/Makefile
>> @@ -1,6 +1,7 @@
>>  subdir-y += svm
>>  subdir-y += vmx
>>
>> +obj-y += acpi.o
>>  obj-y += asid.o
>>  obj-y += emulate.o
>>  obj-y += hpet.o
>> diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
>> new file mode 100644
>> index 0000000..7d42eaf
>> --- /dev/null
>> +++ b/xen/arch/x86/hvm/acpi.c
>> @@ -0,0 +1,24 @@
>> +/* acpi.c: ACPI access handling
>> + *
>> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights
>> reserved.
>> + */
>> +#include <xen/errno.h>
>> +#include <xen/lib.h>
>> +#include <xen/sched.h>
>> +
>> +
>> +int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
>> +                           XEN_GUEST_HANDLE_PARAM(uint8) arg)
>> +{
>> +    return -ENOSYS;
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * tab-width: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/include/asm-x86/hvm/domain.h
>> b/xen/include/asm-x86/hvm/domain.h
>> index d55b432..c5cd86c 100644
>> --- a/xen/include/asm-x86/hvm/domain.h
>> +++ b/xen/include/asm-x86/hvm/domain.h
>> @@ -158,6 +158,9 @@ struct hvm_domain {
>>
>>  #define hap_enabled(d)  ((d)->arch.hvm_domain.hap_enabled)
>>
>> +int hvm_acpi_domctl_access(struct domain *currd, uint8_t rw, gas_t
>> *gas,
>> +                           XEN_GUEST_HANDLE_PARAM(uint8) arg);
>> +
>>  #endif /* __ASM_X86_HVM_DOMAIN_H__ */
>>
>>  /*
>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>> index 177319d..26fe009 100644
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>>
>> +/* ACPI Generic Address Structure */
>> +typedef struct gas {
>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>> +#define XEN_ACPI_SYSTEM_IO     1
>> +    uint8_t    space_id;           /* Address space */
>> +    uint8_t    bit_width;          /* Size in bits of given register */
>> +    uint8_t    bit_offset;         /* Bit offset within the register */
>> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
>> +    uint64_t   address;            /* 64-bit address of register */
>> +} gas_t;
>> +
>> +struct xen_domctl_acpi_access {
>> +    gas_t      gas;                    /* IN: Register being
>> accessed */
>> +
>> +#define XEN_DOMCTL_ACPI_READ   0
>> +#define XEN_DOMCTL_ACPI_WRITE  1
>> +    uint8_t    rw;                     /* IN: Read or write */
>> +
>> +    XEN_GUEST_HANDLE_64(uint8) val;    /* IN/OUT: data */
>> +};
>> +typedef struct xen_domctl_acpi_access xen_domctl_acpi_access_t;
>> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_acpi_access_t);
>> +
>>  struct xen_domctl {
>>      uint32_t cmd;
>>  #define XEN_DOMCTL_createdomain                   1
>> @@ -1221,6 +1244,7 @@ struct xen_domctl {
>>  #define XEN_DOMCTL_monitor_op                    77
>>  #define XEN_DOMCTL_psr_cat_op                    78
>>  #define XEN_DOMCTL_soft_reset                    79
>> +#define XEN_DOMCTL_acpi_access                   80
>>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
>> @@ -1283,6 +1307,7 @@ struct xen_domctl {
>>          struct xen_domctl_psr_cmt_op        psr_cmt_op;
>>          struct xen_domctl_monitor_op        monitor_op;
>>          struct xen_domctl_psr_cat_op        psr_cat_op;
>> +        struct xen_domctl_acpi_access       acpi_access;
>>          uint8_t                             pad[128];
>>      } u;
>>  };
>>
>


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

Re: [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

[Julien Grall]

Hi Boris,

On 12/12/16 16:11, Boris Ostrovsky wrote:
> On 12/12/2016 08:28 AM, Julien Grall wrote:
>> Hi Boris,
>>
>> On 29/11/16 15:33, Boris Ostrovsky wrote:
>>> This domctl will allow toolstack to read and write some
>>> ACPI registers. It will be available to both x86 and ARM
>>> but will be implemented first only for x86
>>
>> Can you explain why we would need this on ARM?
>
> What this is meant to say is that the new domctl is not specific to x86
> and therefore is available as a common (as opposed to arch-specific)
> interface.

I thought you were planning to make this implemented on ARM and was 
trying to understanding why :).

>
> If ARM ever needs to access ACPI space from toostack it should be able
> to use it (provided that an implementation for that is available in
> arch/arm).

Ok. I can't tell whether we would need it in the future. Currently, it 
is not necessary.

Cheers,

-- 
Julien Grall

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

[PATCH v4 07/15] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Boris Ostrovsky]

PVH guests will have ACPI accesses emulated by the hypervisor as
opposed to QEMU (as is the case for HVM guests). This patch installs
handler for accesses to PM1A, GPE0 (which is added to struct
hvm_hw_acpi) and VCPU map. Logic for the handler will be provided
by a later patch.

Whether or not the handler needs to be installed is decided based
on the value of XEN_X86_EMU_ACPI_FF flag which indicates whether
emulation is implemented in QEMU.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
 * Moved handler registration from ioreq.c to acpi.c
 * Renamed XEN_X86_EMU_ACPI_FF to XEN_X86_EMU_ACPI_DM_FF (with a
   corresponding change to has_* macro)

 xen/arch/x86/hvm/acpi.c                | 24 ++++++++++++++++++++++++
 xen/arch/x86/hvm/hvm.c                 |  2 ++
 xen/include/asm-x86/domain.h           |  2 ++
 xen/include/asm-x86/hvm/domain.h       |  1 +
 xen/include/public/arch-x86/hvm/save.h |  2 ++
 xen/include/public/arch-x86/xen.h      |  4 +++-
 6 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index 7d42eaf..a1f7fd2 100644
--- a/xen/arch/x86/hvm/acpi.c
+++ b/xen/arch/x86/hvm/acpi.c
@@ -6,6 +6,7 @@
 #include <xen/lib.h>
 #include <xen/sched.h>
 
+#include <public/arch-x86/xen.h>
 
 int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
                            XEN_GUEST_HANDLE_PARAM(uint8) arg)
@@ -13,6 +14,29 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
     return -ENOSYS;
 }
 
+static int acpi_guest_access(int dir, unsigned int port,
+                             unsigned int bytes, uint32_t *val)
+{
+    return X86EMUL_UNHANDLEABLE;
+}
+
+void hvm_acpi_init(struct domain *d)
+{
+    if ( has_acpi_dm_ff(d) )
+        return;
+
+    register_portio_handler(d, XEN_ACPI_CPU_MAP,
+                            XEN_ACPI_CPU_MAP_LEN, acpi_guest_access);
+    register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1,
+                            sizeof (d->arch.hvm_domain.acpi.gpe0_sts) +
+                            sizeof (d->arch.hvm_domain.acpi.gpe0_en),
+                            acpi_guest_access);
+    register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1,
+                            sizeof (d->arch.hvm_domain.acpi.pm1a_sts) +
+                            sizeof (d->arch.hvm_domain.acpi.pm1a_en),
+                            acpi_guest_access);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 25dc759..9eb518f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -647,6 +647,8 @@ int hvm_domain_initialise(struct domain *d)
 
     hvm_ioreq_init(d);
 
+    hvm_acpi_init(d);
+
     if ( is_pvh_domain(d) )
     {
         register_portio_handler(d, 0, 0x10003, handle_pvh_io);
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index f6a40eb..9509b9e 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -425,6 +425,8 @@ struct arch_domain
 #define has_vvga(d)        (!!((d)->arch.emulation_flags & XEN_X86_EMU_VGA))
 #define has_viommu(d)      (!!((d)->arch.emulation_flags & XEN_X86_EMU_IOMMU))
 #define has_vpit(d)        (!!((d)->arch.emulation_flags & XEN_X86_EMU_PIT))
+#define has_acpi_dm_ff(d)  (!!((d)->arch.emulation_flags & \
+                               XEN_X86_EMU_ACPI_DM_FF))
 
 #define has_arch_pdevs(d)    (!list_empty(&(d)->arch.pdev_list))
 
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index c5cd86c..7ca3b40 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -158,6 +158,7 @@ struct hvm_domain {
 
 #define hap_enabled(d)  ((d)->arch.hvm_domain.hap_enabled)
 
+void hvm_acpi_init(struct domain *d);
 int hvm_acpi_domctl_access(struct domain *currd, uint8_t rw, gas_t *gas,
                            XEN_GUEST_HANDLE_PARAM(uint8) arg);
 
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 3997487..682a738 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -532,6 +532,8 @@ 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 */
+    uint16_t gpe0_sts;
+    uint16_t gpe0_en;
 };
 
 DECLARE_HVM_SAVE_TYPE(ACPI, 13, struct hvm_hw_acpi);
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 0e3a3df..ec5499f 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -283,12 +283,14 @@ struct xen_arch_domainconfig {
 #define XEN_X86_EMU_IOMMU           (1U<<_XEN_X86_EMU_IOMMU)
 #define _XEN_X86_EMU_PIT            8
 #define XEN_X86_EMU_PIT             (1U<<_XEN_X86_EMU_PIT)
+#define _XEN_X86_EMU_ACPI_DM_FF     9
+#define XEN_X86_EMU_ACPI_DM_FF      (1U<<_XEN_X86_EMU_ACPI_DM_FF)
 
 #define XEN_X86_EMU_ALL             (XEN_X86_EMU_LAPIC | XEN_X86_EMU_HPET |  \
                                      XEN_X86_EMU_PM | XEN_X86_EMU_RTC |      \
                                      XEN_X86_EMU_IOAPIC | XEN_X86_EMU_PIC |  \
                                      XEN_X86_EMU_VGA | XEN_X86_EMU_IOMMU |   \
-                                     XEN_X86_EMU_PIT)
+                                     XEN_X86_EMU_PIT | XEN_X86_EMU_ACPI_DM_FF)
     uint32_t emulation_flags;
 };
 
-- 
2.7.4


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

Re: [PATCH v4 07/15] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
> +void hvm_acpi_init(struct domain *d)
> +{
> +    if ( has_acpi_dm_ff(d) )
> +        return;
> +
> +    register_portio_handler(d, XEN_ACPI_CPU_MAP,
> +                            XEN_ACPI_CPU_MAP_LEN, acpi_guest_access);
> +    register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1,
> +                            sizeof (d->arch.hvm_domain.acpi.gpe0_sts) +
> +                            sizeof (d->arch.hvm_domain.acpi.gpe0_en),

Keyword or not, we don't put spaces between sizeof and the
opening parenthesis.

> +                            acpi_guest_access);
> +    register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1,
> +                            sizeof (d->arch.hvm_domain.acpi.pm1a_sts) +
> +                            sizeof (d->arch.hvm_domain.acpi.pm1a_en),
> +                            acpi_guest_access);

Does it really result in most legible / maintainable code (in
subsequent patches) if all three use the same handler?

> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -532,6 +532,8 @@ 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 */
> +    uint16_t gpe0_sts;
> +    uint16_t gpe0_en;
>  };

Don't you need to create compat handling for the case where a
smaller struct arrives during migration? Or do we zero extend
structures nowadays without extra code being needed (assuming
zero extension is what we want in that case in the first place)?

Also the same remark regarding this not being __XEN_TOOLS__
guarded applies here.

Jan


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

Re: [PATCH v4 07/15] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

[Boris Ostrovsky]

On 12/01/2016 11:32 AM, Jan Beulich wrote:
>>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
>> +void hvm_acpi_init(struct domain *d)
>> +{
>> +    if ( has_acpi_dm_ff(d) )
>> +        return;
>> +
>> +    register_portio_handler(d, XEN_ACPI_CPU_MAP,
>> +                            XEN_ACPI_CPU_MAP_LEN, acpi_guest_access);
>> +    register_portio_handler(d, ACPI_GPE0_BLK_ADDRESS_V1,
>> +                            sizeof (d->arch.hvm_domain.acpi.gpe0_sts) +
>> +                            sizeof (d->arch.hvm_domain.acpi.gpe0_en),
> Keyword or not, we don't put spaces between sizeof and the
> opening parenthesis.
>
>> +                            acpi_guest_access);
>> +    register_portio_handler(d, ACPI_PM1A_EVT_BLK_ADDRESS_V1,
>> +                            sizeof (d->arch.hvm_domain.acpi.pm1a_sts) +
>> +                            sizeof (d->arch.hvm_domain.acpi.pm1a_en),
>> +                            acpi_guest_access);
> Does it really result in most legible / maintainable code (in
> subsequent patches) if all three use the same handler?

I can split them into 2 --- one for pm1a/gpe0 and the other for CPU map.
They are indeed handled differently.


>
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -532,6 +532,8 @@ 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 */
>> +    uint16_t gpe0_sts;
>> +    uint16_t gpe0_en;
>>  };
> Don't you need to create compat handling for the case where a
> smaller struct arrives during migration? Or do we zero extend
> structures nowadays without extra code being needed (assuming
> zero extension is what we want in that case in the first place)?

I thought that the fact that new fields are added at the end would make
this safe. And I assumed we will always read into a newly-allocated
hvm_domain, so those registers would be zero.

But being more explicit about this is probably better, I'll add
DECLARE_HVM_SAVE_TYPE_COMPAT.

Andrew, will this interfere with your hypervisor migration v2 plan?


-boris


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

[PATCH v4 08/15] pvh/acpi: Handle ACPI accesses for PVH guests

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
* Registers are now explicitly split into 2-byte status and enable.

 xen/arch/x86/hvm/acpi.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++-
 xen/common/domctl.c     |   5 ++
 xen/include/xen/sched.h |   3 ++
 3 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index a1f7fd2..c80c464 100644
--- a/xen/arch/x86/hvm/acpi.c
+++ b/xen/arch/x86/hvm/acpi.c
@@ -2,12 +2,129 @@
  *
  * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
  */
+#include <xen/acpi.h>
 #include <xen/errno.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
 
 #include <public/arch-x86/xen.h>
 
+static int acpi_access_common(struct domain *d,
+                              int dir, unsigned int port,
+                              unsigned int bytes, uint32_t *val)
+{
+    uint16_t *sts = NULL, *en = NULL;
+    const uint16_t *mask_sts = NULL, *mask_en = NULL;
+    static const uint16_t pm1a_sts_mask = ACPI_BITMASK_GLOBAL_LOCK_STATUS;
+    static const uint16_t pm1a_en_mask = ACPI_BITMASK_GLOBAL_LOCK_ENABLE;
+    static const uint16_t gpe0_sts_mask = 1U << XEN_GPE0_CPUHP_BIT;
+    static const uint16_t gpe0_en_mask = 1U << XEN_GPE0_CPUHP_BIT;
+
+    BUILD_BUG_ON(XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN
+                 >= ACPI_GPE0_BLK_ADDRESS_V1);
+
+    ASSERT(!has_acpi_dm_ff(d));
+
+    switch ( port )
+    {
+    case ACPI_PM1A_EVT_BLK_ADDRESS_V1 ...
+        ACPI_PM1A_EVT_BLK_ADDRESS_V1 +
+        sizeof (d->arch.hvm_domain.acpi.pm1a_sts) +
+        sizeof (d->arch.hvm_domain.acpi.pm1a_en):
+
+        sts = &d->arch.hvm_domain.acpi.pm1a_sts;
+        en = &d->arch.hvm_domain.acpi.pm1a_en;
+        mask_sts = &pm1a_sts_mask;
+        mask_en = &pm1a_en_mask;
+        break;
+
+    case ACPI_GPE0_BLK_ADDRESS_V1 ...
+        ACPI_GPE0_BLK_ADDRESS_V1 +
+        sizeof (d->arch.hvm_domain.acpi.gpe0_sts) +
+        sizeof (d->arch.hvm_domain.acpi.gpe0_en):
+
+        sts = &d->arch.hvm_domain.acpi.gpe0_sts;
+        en = &d->arch.hvm_domain.acpi.gpe0_en;
+        mask_sts = &gpe0_sts_mask;
+        mask_en = &gpe0_en_mask;
+        break;
+
+    case XEN_ACPI_CPU_MAP ...
+         XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN - 1:
+        break;
+
+    default:
+        return X86EMUL_UNHANDLEABLE;
+    }
+
+    if ( dir == XEN_DOMCTL_ACPI_READ )
+    {
+        uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
+
+        if ( !mask_sts )
+        {
+            unsigned int first_byte = port - XEN_ACPI_CPU_MAP;
+
+            /*
+             * Clear bits that we are about to read to in case we
+             * copy fewer than @bytes.
+             */
+            *val &= mask;
+
+            if ( ((d->max_vcpus + 7) / 8) > first_byte )
+            {
+                memcpy(val, (uint8_t *)d->avail_vcpus + first_byte,
+                       min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
+            }
+        }
+        else
+        {
+            uint32_t data = (((uint32_t)*en) << 16) | *sts;
+            data >>= 8 * (port & 3);
+            *val = (*val & mask) | (data & ~mask);
+        }
+    }
+    else
+    {
+        /* Guests do not write CPU map */
+        if ( !mask_sts )
+            return X86EMUL_UNHANDLEABLE;
+        else if ( mask_sts )
+        {
+            uint32_t v = *val;
+
+            /* Status register is write-1-to-clear by guests */
+            switch ( port & 3 )
+            {
+            case 0:
+                *sts &= ~(v & 0xff);
+                *sts &= *mask_sts;
+                if ( !--bytes )
+                    break;
+                v >>= 8;
+
+            case 1:
+                *sts &= ~((v & 0xff) << 8);
+                *sts &= *mask_sts;
+                if ( !--bytes )
+                    break;
+                v >>= 8;
+
+            case 2:
+                *en = ((*en & 0xff00) | (v & 0xff)) & *mask_en;
+                if ( !--bytes )
+                    break;
+                v >>= 8;
+
+            case 3:
+                *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
+            }
+        }
+    }
+
+    return X86EMUL_OKAY;
+}
+
 int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
                            XEN_GUEST_HANDLE_PARAM(uint8) arg)
 {
@@ -17,7 +134,10 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
 static int acpi_guest_access(int dir, unsigned int port,
                              unsigned int bytes, uint32_t *val)
 {
-    return X86EMUL_UNHANDLEABLE;
+    return  acpi_access_common(current->domain,
+                               (dir == IOREQ_READ) ?
+                               XEN_DOMCTL_ACPI_READ: XEN_DOMCTL_ACPI_WRITE,
+                               port, bytes, val);
 }
 
 void hvm_acpi_init(struct domain *d)
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index b0ee961..0a08b83 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -651,6 +651,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
                 goto maxvcpu_out;
         }
 
+        d->avail_vcpus = xzalloc_array(unsigned long,
+                                       BITS_TO_LONGS(d->max_vcpus));
+        if ( !d->avail_vcpus )
+            goto maxvcpu_out;
+
         ret = 0;
 
     maxvcpu_out:
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 1fbda87..3ef9c9e 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -315,6 +315,9 @@ struct domain
     unsigned int     max_vcpus;
     struct vcpu    **vcpu;
 
+    /* Bitmap of available VCPUs. */
+    unsigned long   *avail_vcpus;
+
     shared_info_t   *shared_info;     /* shared data area */
 
     spinlock_t       domain_lock;
-- 
2.7.4


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

Re: [PATCH v4 08/15] pvh/acpi: Handle ACPI accesses for PVH guests

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
> +static int acpi_access_common(struct domain *d,
> +                              int dir, unsigned int port,
> +                              unsigned int bytes, uint32_t *val)
> +{

Why is this a separate function instead of the body of
acpi_guest_access()? Do you mean to later use this for the
domctl handling (as the use of XEN_DOMCTL_ACPI_* suggests)?
Such things may be worthwhile mentioning at least after the first
--- marker.

> +    uint16_t *sts = NULL, *en = NULL;
> +    const uint16_t *mask_sts = NULL, *mask_en = NULL;
> +    static const uint16_t pm1a_sts_mask = ACPI_BITMASK_GLOBAL_LOCK_STATUS;
> +    static const uint16_t pm1a_en_mask = ACPI_BITMASK_GLOBAL_LOCK_ENABLE;
> +    static const uint16_t gpe0_sts_mask = 1U << XEN_GPE0_CPUHP_BIT;
> +    static const uint16_t gpe0_en_mask = 1U << XEN_GPE0_CPUHP_BIT;
> +
> +    BUILD_BUG_ON(XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN
> +                 >= ACPI_GPE0_BLK_ADDRESS_V1);
> +
> +    ASSERT(!has_acpi_dm_ff(d));
> +
> +    switch ( port )
> +    {
> +    case ACPI_PM1A_EVT_BLK_ADDRESS_V1 ...
> +        ACPI_PM1A_EVT_BLK_ADDRESS_V1 +
> +        sizeof (d->arch.hvm_domain.acpi.pm1a_sts) +
> +        sizeof (d->arch.hvm_domain.acpi.pm1a_en):

Same remark as for an earlier patch regarding the blanks here.

> +        sts = &d->arch.hvm_domain.acpi.pm1a_sts;
> +        en = &d->arch.hvm_domain.acpi.pm1a_en;
> +        mask_sts = &pm1a_sts_mask;
> +        mask_en = &pm1a_en_mask;
> +        break;
> +
> +    case ACPI_GPE0_BLK_ADDRESS_V1 ...
> +        ACPI_GPE0_BLK_ADDRESS_V1 +
> +        sizeof (d->arch.hvm_domain.acpi.gpe0_sts) +
> +        sizeof (d->arch.hvm_domain.acpi.gpe0_en):
> +
> +        sts = &d->arch.hvm_domain.acpi.gpe0_sts;
> +        en = &d->arch.hvm_domain.acpi.gpe0_en;
> +        mask_sts = &gpe0_sts_mask;
> +        mask_en = &gpe0_en_mask;
> +        break;
> +
> +    case XEN_ACPI_CPU_MAP ...
> +         XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN - 1:
> +        break;
> +
> +    default:
> +        return X86EMUL_UNHANDLEABLE;
> +    }
> +
> +    if ( dir == XEN_DOMCTL_ACPI_READ )
> +    {
> +        uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
> +
> +        if ( !mask_sts )
> +        {
> +            unsigned int first_byte = port - XEN_ACPI_CPU_MAP;
> +
> +            /*
> +             * Clear bits that we are about to read to in case we
> +             * copy fewer than @bytes.
> +             */
> +            *val &= mask;
> +
> +            if ( ((d->max_vcpus + 7) / 8) > first_byte )
> +            {
> +                memcpy(val, (uint8_t *)d->avail_vcpus + first_byte,
> +                       min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
> +            }

Unnecessary braces.

> +        }
> +        else
> +        {
> +            uint32_t data = (((uint32_t)*en) << 16) | *sts;
> +            data >>= 8 * (port & 3);

Blank line between declaration and statement(s) please.

> +            *val = (*val & mask) | (data & ~mask);
> +        }
> +    }
> +    else
> +    {
> +        /* Guests do not write CPU map */
> +        if ( !mask_sts )
> +            return X86EMUL_UNHANDLEABLE;
> +        else if ( mask_sts )
> +        {
> +            uint32_t v = *val;
> +
> +            /* Status register is write-1-to-clear by guests */
> +            switch ( port & 3 )
> +            {
> +            case 0:
> +                *sts &= ~(v & 0xff);
> +                *sts &= *mask_sts;
> +                if ( !--bytes )
> +                    break;
> +                v >>= 8;
> +
> +            case 1:
> +                *sts &= ~((v & 0xff) << 8);
> +                *sts &= *mask_sts;
> +                if ( !--bytes )
> +                    break;
> +                v >>= 8;
> +
> +            case 2:
> +                *en = ((*en & 0xff00) | (v & 0xff)) & *mask_en;
> +                if ( !--bytes )
> +                    break;
> +                v >>= 8;
> +
> +            case 3:
> +                *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
> +            }

Please annotate intended fall-through with comments, to silence
Coverity. Also the last one would better end with break.

> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -651,6 +651,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>                  goto maxvcpu_out;
>          }
>  
> +        d->avail_vcpus = xzalloc_array(unsigned long,
> +                                       BITS_TO_LONGS(d->max_vcpus));
> +        if ( !d->avail_vcpus )
> +            goto maxvcpu_out;

Considering this isn't being touched outside of
acpi_access_common(), how come you get away without setting
the bits for the vCPU-s online when the guest starts?

Also you appear to leak this array when the domain gets destroyed.

Jan

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

Re: [PATCH v4 08/15] pvh/acpi: Handle ACPI accesses for PVH guests

[Boris Ostrovsky]

On 12/06/2016 09:34 AM, Jan Beulich wrote:
>>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
>> +static int acpi_access_common(struct domain *d,
>> +                              int dir, unsigned int port,
>> +                              unsigned int bytes, uint32_t *val)
>> +{
> Why is this a separate function instead of the body of
> acpi_guest_access()? Do you mean to later use this for the
> domctl handling (as the use of XEN_DOMCTL_ACPI_* suggests)?
> Such things may be worthwhile mentioning at least after the first
> --- marker.

Yes, this becomes common code with the subsequent patch. I'll note this
in the commit message.


>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -651,6 +651,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>                  goto maxvcpu_out;
>>          }
>>  
>> +        d->avail_vcpus = xzalloc_array(unsigned long,
>> +                                       BITS_TO_LONGS(d->max_vcpus));
>> +        if ( !d->avail_vcpus )
>> +            goto maxvcpu_out;
> Considering this isn't being touched outside of
> acpi_access_common(), how come you get away without setting
> the bits for the vCPU-s online when the guest starts?

The AML code, which is the only reader of the map, is only executed
after the map has been updated (i.e. the SCI has been sent). But I
probably should have the toolstack initialize it when building the guest.

-boris


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

Re: [PATCH v4 08/15] pvh/acpi: Handle ACPI accesses for PVH guests

[Jan Beulich]

>>> On 06.12.16 at 17:37, <boris.ostrovsky@oracle.com> wrote:
> On 12/06/2016 09:34 AM, Jan Beulich wrote:
>>>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
>>> --- a/xen/common/domctl.c
>>> +++ b/xen/common/domctl.c
>>> @@ -651,6 +651,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>>                  goto maxvcpu_out;
>>>          }
>>>  
>>> +        d->avail_vcpus = xzalloc_array(unsigned long,
>>> +                                       BITS_TO_LONGS(d->max_vcpus));
>>> +        if ( !d->avail_vcpus )
>>> +            goto maxvcpu_out;
>> Considering this isn't being touched outside of
>> acpi_access_common(), how come you get away without setting
>> the bits for the vCPU-s online when the guest starts?
> 
> The AML code, which is the only reader of the map, is only executed
> after the map has been updated (i.e. the SCI has been sent). But I
> probably should have the toolstack initialize it when building the guest.

Thanks - that'll prevent this becoming a latent bug should some
other consumer of the map appear.

Jan


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

[PATCH v4 09/15] x86/domctl: Handle ACPI access from domctl

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
* New patch

 xen/arch/x86/hvm/acpi.c | 61 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 54 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index c80c464..1c205eb 100644
--- a/xen/arch/x86/hvm/acpi.c
+++ b/xen/arch/x86/hvm/acpi.c
@@ -7,9 +7,11 @@
 #include <xen/lib.h>
 #include <xen/sched.h>
 
+#include <asm/guest_access.h>
+
 #include <public/arch-x86/xen.h>
 
-static int acpi_access_common(struct domain *d,
+static int acpi_access_common(struct domain *d, bool is_guest_access,
                               int dir, unsigned int port,
                               unsigned int bytes, uint32_t *val)
 {
@@ -87,8 +89,13 @@ static int acpi_access_common(struct domain *d,
     else
     {
         /* Guests do not write CPU map */
-        if ( !mask_sts )
-            return X86EMUL_UNHANDLEABLE;
+        if ( !is_guest_access && !mask_sts )
+        {
+            unsigned int first_byte = port - XEN_ACPI_CPU_MAP;
+
+            memcpy((uint8_t *)d->avail_vcpus + first_byte, val,
+                   min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
+        }
         else if ( mask_sts )
         {
             uint32_t v = *val;
@@ -97,14 +104,20 @@ static int acpi_access_common(struct domain *d,
             switch ( port & 3 )
             {
             case 0:
-                *sts &= ~(v & 0xff);
+                if ( is_guest_access )
+                    *sts &= ~(v & 0xff);
+                else
+                    *sts = (*sts & 0xff00) | (v & 0xff);
                 *sts &= *mask_sts;
                 if ( !--bytes )
                     break;
                 v >>= 8;
 
             case 1:
-                *sts &= ~((v & 0xff) << 8);
+                if ( is_guest_access )
+                    *sts &= ~((v & 0xff) << 8);
+                else
+                    *sts = ((v & 0xff) << 8) | (*sts & 0xff);
                 *sts &= *mask_sts;
                 if ( !--bytes )
                     break;
@@ -128,13 +141,47 @@ static int acpi_access_common(struct domain *d,
 int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,
                            XEN_GUEST_HANDLE_PARAM(uint8) arg)
 {
-    return -ENOSYS;
+    unsigned int port, tot_bytes, bytes;
+    unsigned int i;
+    uint32_t val;
+    uint8_t *ptr = (uint8_t *)&val;
+    int rc;
+
+    /*
+     * Only allow accesses to IO space. Accesses should be byte-aligned
+     * and integral number of bytes.
+     */
+    if ( gas->space_id != XEN_ACPI_SYSTEM_IO ||
+         gas->bit_width & 7 || gas->bit_offset & 7 )
+        return -EINVAL;
+
+    port = gas->address + (gas->bit_offset >> 3);
+    tot_bytes = gas->bit_width >> 3;
+
+    for ( i = 0; i < tot_bytes; i += sizeof(val) )
+    {
+        bytes = (tot_bytes - i > sizeof(val)) ? sizeof(val) : tot_bytes - i;
+
+        if ((rw == XEN_DOMCTL_ACPI_WRITE) &&
+            copy_from_guest_offset(ptr, arg, i, bytes))
+            return -EFAULT;
+
+        rc = acpi_access_common(d, false, rw, port + i, bytes, &val);
+        if ( rc )
+            return rc;
+
+        if ((rw == XEN_DOMCTL_ACPI_READ) &&
+            copy_to_guest_offset(arg, i, ptr, bytes))
+            return -EFAULT;
+    }
+
+    return 0;
 }
 
 static int acpi_guest_access(int dir, unsigned int port,
                              unsigned int bytes, uint32_t *val)
 {
-    return  acpi_access_common(current->domain,
+    return  acpi_access_common(current->domain, true,
                                (dir == IOREQ_READ) ?
                                XEN_DOMCTL_ACPI_READ: XEN_DOMCTL_ACPI_WRITE,
                                port, bytes, val);
-- 
2.7.4


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

[PATCH v4 10/15] events/x86: Define SCI virtual interrupt

[Boris Ostrovsky]

PVH guests do not have IOAPIC which typically generates an SCI. For
those guests SCI will be provided as a virtual interrupt.

Copy VIRQ_MCA definition from of xen-mca.h to xen.h to keep all
x86-specific VIRQ_ARCH_* in one place. (However, because we don't
want to require inclusion of xen.h in xen-mca.h we preserve original
definition of VIRQ_MCA as well.)

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
---
Changes in v4:
* Kept VIRQ_MCA definition in xen-mca.h (TBH though I am not convinced
  this is better than including xen.h there)

 xen/include/public/arch-x86/xen.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index ec5499f..619cd48 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -302,6 +302,9 @@ struct xen_arch_domainconfig {
 #define XEN_GPE0_CPUHP_BIT           2
 #endif
 
+#define VIRQ_MCA VIRQ_ARCH_0 /* G. (DOM0) Machine Check Architecture */
+#define VIRQ_SCI VIRQ_ARCH_1 /* G. (PVH) ACPI interrupt */
+
 #endif /* !__ASSEMBLY__ */
 
 /*
-- 
2.7.4


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

Re: [PATCH v4 10/15] events/x86: Define SCI virtual interrupt

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
> PVH guests do not have IOAPIC which typically generates an SCI. For
> those guests SCI will be provided as a virtual interrupt.
> 
> Copy VIRQ_MCA definition from of xen-mca.h to xen.h to keep all
> x86-specific VIRQ_ARCH_* in one place. (However, because we don't
> want to require inclusion of xen.h in xen-mca.h we preserve original
> definition of VIRQ_MCA as well.)
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


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

[PATCH v4 11/15] pvh: Send an SCI on VCPU hotplug event

[Boris Ostrovsky]

When GPE0 status register gets a bit set (currently XEN_GPE0_CPUHP_BIT
only) send an SCI to the guest.

Also update send_guest_global_virq() to handle cases when VCPU0
is offlined.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
* Generate SCI only when needed (i.e. when status changes)
* Deal with VCPU0 being offline

 xen/arch/x86/hvm/acpi.c    | 10 ++++++++++
 xen/common/event_channel.c |  7 +++++--
 xen/include/xen/domain.h   |  1 +
 xen/include/xen/event.h    |  8 ++++++++
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index 1c205eb..9b11a52 100644
--- a/xen/arch/x86/hvm/acpi.c
+++ b/xen/arch/x86/hvm/acpi.c
@@ -4,6 +4,7 @@
  */
 #include <xen/acpi.h>
 #include <xen/errno.h>
+#include <xen/event.h>
 #include <xen/lib.h>
 #include <xen/sched.h>
 
@@ -99,6 +100,7 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
         else if ( mask_sts )
         {
             uint32_t v = *val;
+            uint16_t sts_orig = *sts;
 
             /* Status register is write-1-to-clear by guests */
             switch ( port & 3 )
@@ -132,6 +134,14 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
             case 3:
                 *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
             }
+
+            /*
+             * If a new bit has been set in status register and corresponding
+             * event is enabled then an SCI is sent to the guest.
+             */
+            if ( !is_guest_access &&
+                 ((*sts ^ sts_orig) & ~sts_orig) && (*sts & *en))
+                send_guest_global_virq(d, VIRQ_SCI);
         }
     }
 
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 638dc5e..1d77373 100644
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -727,7 +727,7 @@ void send_guest_vcpu_virq(struct vcpu *v, uint32_t virq)
     spin_unlock_irqrestore(&v->virq_lock, flags);
 }
 
-static void send_guest_global_virq(struct domain *d, uint32_t virq)
+void send_guest_global_virq(struct domain *d, uint32_t virq)
 {
     unsigned long flags;
     int port;
@@ -739,7 +739,10 @@ static void send_guest_global_virq(struct domain *d, uint32_t virq)
     if ( unlikely(d == NULL) || unlikely(d->vcpu == NULL) )
         return;
 
-    v = d->vcpu[0];
+    /* Send to first available VCPU */
+    for_each_vcpu(d, v)
+        if ( is_vcpu_online(v) )
+            break;
     if ( unlikely(v == NULL) )
         return;
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index bce0ea1..b386038 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -52,6 +52,7 @@ void vcpu_destroy(struct vcpu *v);
 int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
 void unmap_vcpu_info(struct vcpu *v);
 
+int arch_update_avail_vcpus(struct domain *d);
 int arch_domain_create(struct domain *d, unsigned int domcr_flags,
                        struct xen_arch_domainconfig *config);
 
diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
index 5008c80..74bd605 100644
--- a/xen/include/xen/event.h
+++ b/xen/include/xen/event.h
@@ -23,6 +23,14 @@
 void send_guest_vcpu_virq(struct vcpu *v, uint32_t virq);
 
 /*
+ * send_guest_global_virq: Notify guest via a global VIRQ.
+ *  @d:        domain to which virtual IRQ should be sent. First
+ *             online VCPU will be selected.
+ *  @virq:     Virtual IRQ number (VIRQ_*)
+ */
+void send_guest_global_virq(struct domain *d, uint32_t virq);
+
+/*
  * send_global_virq: Notify the domain handling a global VIRQ.
  *  @virq:     Virtual IRQ number (VIRQ_*)
  */
-- 
2.7.4


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

Re: [PATCH v4 11/15] pvh: Send an SCI on VCPU hotplug event

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
> @@ -99,6 +100,7 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
>          else if ( mask_sts )
>          {
>              uint32_t v = *val;
> +            uint16_t sts_orig = *sts;
>  
>              /* Status register is write-1-to-clear by guests */
>              switch ( port & 3 )
> @@ -132,6 +134,14 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
>              case 3:
>                  *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
>              }
> +
> +            /*
> +             * If a new bit has been set in status register and corresponding
> +             * event is enabled then an SCI is sent to the guest.
> +             */
> +            if ( !is_guest_access &&
> +                 ((*sts ^ sts_orig) & ~sts_orig) && (*sts & *en))
> +                send_guest_global_virq(d, VIRQ_SCI);

I don't think comment and condition match; namely the
"corresponding" doesn't appear to be fulfilled - you raise
SCI if any bit is set both in *sts and *en.

Also (a ^ b) & ~b = a & ~b afaict.

Jan


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

Re: [PATCH v4 11/15] pvh: Send an SCI on VCPU hotplug event

[Boris Ostrovsky]

On 12/06/2016 09:50 AM, Jan Beulich wrote:
>>
>> +
>> +            /*
>> +             * If a new bit has been set in status register and corresponding
>> +             * event is enabled then an SCI is sent to the guest.
>> +             */
>> +            if ( !is_guest_access &&
>> +                 ((*sts ^ sts_orig) & ~sts_orig) && (*sts & *en))
>> +                send_guest_global_virq(d, VIRQ_SCI);
> I don't think comment and condition match; namely the
> "corresponding" doesn't appear to be fulfilled - you raise
> SCI if any bit is set both in *sts and *en.

Oh, that was just wrong. Should be (taking your simplification below in
to account)
    (*sts & ~sts_orig) & *en

-boris

>
> Also (a ^ b) & ~b = a & ~b afaict.
>
> Jan
>


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

[PATCH v4 12/15] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug

[Boris Ostrovsky]

Provide libxc interface for accessing ACPI via XEN_DOMCTL_acpi_access.

When a VCPU is hot-(un)plugged to/from a PVH guest update VCPU map
by writing to ACPI's XEN_ACPI_CPU_MAP register and then set GPE0
status bit in GPE0.status.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
* New patch

 tools/libxc/include/xenctrl.h | 20 ++++++++++++++++++++
 tools/libxc/xc_domain.c       | 36 ++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl.c           |  8 +++++++-
 tools/libxl/libxl_arch.h      |  4 ++++
 tools/libxl/libxl_arm.c       |  6 ++++++
 tools/libxl/libxl_x86.c       | 21 +++++++++++++++++++++
 6 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 2c83544..e4d735f 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2710,6 +2710,26 @@ int xc_livepatch_revert(xc_interface *xch, char *name, uint32_t timeout);
 int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout);
 int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout);
 
+int xc_acpi_access(xc_interface *xch, domid_t domid,
+                   uint8_t rw, uint8_t space_id, unsigned long addr,
+                   unsigned int bytes, void *val);
+
+static inline int xc_acpi_ioread(xc_interface *xch, domid_t domid,
+                                 unsigned long port,
+                                 unsigned int bytes, void *val)
+{
+    return xc_acpi_access(xch, domid, XEN_DOMCTL_ACPI_READ, XEN_ACPI_SYSTEM_IO,
+                          port, bytes, val);
+}
+
+static inline int xc_acpi_iowrite(xc_interface *xch, domid_t domid,
+                                  unsigned long port,
+                                  unsigned int bytes, void *val)
+{
+    return xc_acpi_access(xch, domid, XEN_DOMCTL_ACPI_WRITE, XEN_ACPI_SYSTEM_IO,
+                          port, bytes, val);
+}
+
 /* Compat shims */
 #include "xenctrl_compat.h"
 
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 296b852..15c5136 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2520,6 +2520,42 @@ int xc_domain_soft_reset(xc_interface *xch,
     domctl.domain = (domid_t)domid;
     return do_domctl(xch, &domctl);
 }
+
+int
+xc_acpi_access(xc_interface *xch, domid_t domid,
+               uint8_t rw, uint8_t space_id,
+               unsigned long address, unsigned int bytes, void *val)
+{
+    DECLARE_DOMCTL;
+    DECLARE_HYPERCALL_BOUNCE(val, bytes, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+
+    while ( (int)bytes > 0 )
+    {
+        if ( xc_hypercall_bounce_pre(xch, val) )
+            return -1;
+
+        memset(&domctl, 0, sizeof(domctl));
+        domctl.domain = domid;
+        domctl.cmd = XEN_DOMCTL_acpi_access;
+        domctl.u.acpi_access.gas.space_id = space_id;
+        domctl.u.acpi_access.gas.bit_width = (bytes & 31) * 8;
+        domctl.u.acpi_access.gas.bit_offset = 0;
+        domctl.u.acpi_access.gas.address = address;
+        domctl.u.acpi_access.rw = rw;
+        set_xen_guest_handle(domctl.u.acpi_access.val, val);
+
+        if ( do_domctl(xch, &domctl) != 0 )
+            return 1;
+
+        xc_hypercall_bounce_post(xch, val);
+
+        bytes -= 32;
+        address += 32;
+    }
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 33c5e4c..d80ab77 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5147,8 +5147,14 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
         switch (libxl__device_model_version_running(gc, domid)) {
-        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
         case LIBXL_DEVICE_MODEL_VERSION_NONE:
+            rc = libxl__arch_set_vcpuonline(gc, domid, cpumap);
+            if (rc < 0) {
+                LOGE(ERROR, "Can't change vcpu online map (%d)", rc);
+                goto out;
+            }
+            /* fallthrough */
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
             rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
             break;
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index 5e1fc60..1869626 100644
--- a/tools/libxl/libxl_arch.h
+++ b/tools/libxl/libxl_arch.h
@@ -71,6 +71,10 @@ int libxl__arch_extra_memory(libxl__gc *gc,
                              const libxl_domain_build_info *info,
                              uint64_t *out);
 
+_hidden
+int libxl__arch_set_vcpuonline(libxl__gc *gc, uint32_t domid,
+			       libxl_bitmap *cpumap);
+
 #if defined(__i386__) || defined(__x86_64__)
 
 #define LAPIC_BASE_ADDRESS  0xfee00000
diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index d842d88..a64af1b 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -126,6 +126,12 @@ out:
     return rc;
 }
 
+int libxl__arch_set_vcpuonline(libxl__gc *gc, uint32_t domid,
+			       libxl_bitmap *cpumap)
+{
+    return ERROR_FAIL;
+}
+
 static struct arch_info {
     const char *guest_type;
     const char *timer_compat;
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index e1844c8..e31b159 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -3,6 +3,9 @@
 
 #include <xc_dom.h>
 
+#include <xen/arch-x86/xen.h>
+#include <xen/hvm/ioreq.h>
+
 int libxl__arch_domain_prepare_config(libxl__gc *gc,
                                       libxl_domain_config *d_config,
                                       xc_domain_configuration_t *xc_config)
@@ -368,6 +371,24 @@ int libxl__arch_extra_memory(libxl__gc *gc,
     return 0;
 }
 
+int libxl__arch_set_vcpuonline(libxl__gc *gc, uint32_t domid,
+			       libxl_bitmap *cpumap)
+{
+    int rc;
+
+    /*Update VCPU map. */
+    rc = xc_acpi_iowrite(CTX->xch, domid, XEN_ACPI_CPU_MAP,
+                         cpumap->size, cpumap->map);
+    if (!rc) {
+        /* Send an SCI. */
+        uint16_t val = 1 << XEN_GPE0_CPUHP_BIT;
+        rc = xc_acpi_iowrite(CTX->xch, domid, ACPI_GPE0_BLK_ADDRESS_V1,
+                             sizeof(val), &val);
+    }
+
+    return rc;
+}
+
 int libxl__arch_domain_init_hw_description(libxl__gc *gc,
                                            libxl_domain_build_info *info,
                                            libxl__domain_build_state *state,
-- 
2.7.4


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

Re: [PATCH v4 12/15] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug

[Wei Liu]

On Tue, Nov 29, 2016 at 10:33:19AM -0500, Boris Ostrovsky wrote:
[...]
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 33c5e4c..d80ab77 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5147,8 +5147,14 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
>      switch (libxl__domain_type(gc, domid)) {
>      case LIBXL_DOMAIN_TYPE_HVM:
>          switch (libxl__device_model_version_running(gc, domid)) {
> -        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
>          case LIBXL_DEVICE_MODEL_VERSION_NONE:
> +            rc = libxl__arch_set_vcpuonline(gc, domid, cpumap);
> +            if (rc < 0) {
> +                LOGE(ERROR, "Can't change vcpu online map (%d)", rc);
> +                goto out;
> +            }
> +            /* fallthrough */
> +        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:

What is this "fallthrough"? I don't see description for this part in
your doc later.

>              rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
>              break;
>          case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
> index 5e1fc60..1869626 100644
> --- a/tools/libxl/libxl_arch.h
> +++ b/tools/libxl/libxl_arch.h
> @@ -71,6 +71,10 @@ int libxl__arch_extra_memory(libxl__gc *gc,
>                               const libxl_domain_build_info *info,
>                               uint64_t *out);
>  
> +_hidden
> +int libxl__arch_set_vcpuonline(libxl__gc *gc, uint32_t domid,
> +			       libxl_bitmap *cpumap);

No tabs please.

Wei.

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

Re: [PATCH v4 12/15] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug

[Boris Ostrovsky]

On 12/12/2016 11:35 AM, Wei Liu wrote:
> On Tue, Nov 29, 2016 at 10:33:19AM -0500, Boris Ostrovsky wrote:
> [...]
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 33c5e4c..d80ab77 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -5147,8 +5147,14 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
>>      switch (libxl__domain_type(gc, domid)) {
>>      case LIBXL_DOMAIN_TYPE_HVM:
>>          switch (libxl__device_model_version_running(gc, domid)) {
>> -        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
>>          case LIBXL_DEVICE_MODEL_VERSION_NONE:
>> +            rc = libxl__arch_set_vcpuonline(gc, domid, cpumap);
>> +            if (rc < 0) {
>> +                LOGE(ERROR, "Can't change vcpu online map (%d)", rc);
>> +                goto out;
>> +            }
>> +            /* fallthrough */
>> +        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> What is this "fallthrough"? I don't see description for this part in
> your doc later.

It is not required for VCPU hotplug to function.I do this for
consistency because otherwise even after hotplug the VCPU is still
marked as offline in xenstore.

Come think of it, maybe we should do this for all guests (which really
means for HVM too)?

-boris

>
>>              rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
>>              break;
>>          case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
>> index 5e1fc60..1869626 100644
>> --- a/tools/libxl/libxl_arch.h
>> +++ b/tools/libxl/libxl_arch.h
>> @@ -71,6 +71,10 @@ int libxl__arch_extra_memory(libxl__gc *gc,
>>                               const libxl_domain_build_info *info,
>>                               uint64_t *out);
>>  
>> +_hidden
>> +int libxl__arch_set_vcpuonline(libxl__gc *gc, uint32_t domid,
>> +			       libxl_bitmap *cpumap);
> No tabs please.
>
> Wei.


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

Re: [PATCH v4 12/15] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug

[Boris Ostrovsky]

On 12/12/2016 11:47 AM, Boris Ostrovsky wrote:
>
> Come think of it, maybe we should do this for all guests (which really
> means for HVM too)?

I meant, of course, for qemu-upstream HVM guests too.

-boris



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

Re: [PATCH v4 12/15] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug

[Wei Liu]

On Mon, Dec 12, 2016 at 11:50:53AM -0500, Boris Ostrovsky wrote:
> On 12/12/2016 11:47 AM, Boris Ostrovsky wrote:
> >
> > Come think of it, maybe we should do this for all guests (which really
> > means for HVM too)?
> 
> I meant, of course, for qemu-upstream HVM guests too.
> 

Fine by me -- but please add a line of comment before the "fallthrough"
to say that this is just indicative.

Wei.

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

Re: [PATCH v4 12/15] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug

[Boris Ostrovsky]

On 12/12/2016 12:09 PM, Wei Liu wrote:
> On Mon, Dec 12, 2016 at 11:50:53AM -0500, Boris Ostrovsky wrote:
>> On 12/12/2016 11:47 AM, Boris Ostrovsky wrote:
>>> Come think of it, maybe we should do this for all guests (which really
>>> means for HVM too)?
>> I meant, of course, for qemu-upstream HVM guests too.
>>
> Fine by me -- but please add a line of comment before the "fallthrough"
> to say that this is just indicative.
>

I think making xenstore update for all guests would be better done in a
patch of its own since it really doesn't have anything to do with PVH.

-boris

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

Re: [PATCH v4 12/15] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug

[Wei Liu]

On Mon, Dec 12, 2016 at 12:14:36PM -0500, Boris Ostrovsky wrote:
> On 12/12/2016 12:09 PM, Wei Liu wrote:
> > On Mon, Dec 12, 2016 at 11:50:53AM -0500, Boris Ostrovsky wrote:
> >> On 12/12/2016 11:47 AM, Boris Ostrovsky wrote:
> >>> Come think of it, maybe we should do this for all guests (which really
> >>> means for HVM too)?
> >> I meant, of course, for qemu-upstream HVM guests too.
> >>
> > Fine by me -- but please add a line of comment before the "fallthrough"
> > to say that this is just indicative.
> >
> 
> I think making xenstore update for all guests would be better done in a
> patch of its own since it really doesn't have anything to do with PVH.
> 

That's fine too.

With the tabs removed:

Acked-by: Wei Liu <wei.liu2@citrix.com>

> -boris

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

[PATCH v4 13/15] pvh: Set online VCPU map to avail_vcpus

[Boris Ostrovsky]

ACPI builder marks VCPUS set in vcpu_online map as enabled in MADT.
With ACPI-based CPU hotplug we only want VCPUs that are started by
the guest to be marked as such. Remaining VCPUs will be set to
"enable" by AML code during hotplug.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_x86_acpi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_x86_acpi.c b/tools/libxl/libxl_x86_acpi.c
index ff0e2df..949f555 100644
--- a/tools/libxl/libxl_x86_acpi.c
+++ b/tools/libxl/libxl_x86_acpi.c
@@ -98,7 +98,7 @@ static int init_acpi_config(libxl__gc *gc,
     uint32_t domid = dom->guest_domid;
     xc_dominfo_t info;
     struct hvm_info_table *hvminfo;
-    int i, rc = 0;
+    int rc = 0;
 
     config->dsdt_anycpu = config->dsdt_15cpu = dsdt_pvh;
     config->dsdt_anycpu_len = config->dsdt_15cpu_len = dsdt_pvh_len;
@@ -144,8 +144,8 @@ static int init_acpi_config(libxl__gc *gc,
         hvminfo->nr_vcpus = info.max_vcpu_id + 1;
     }
 
-    for (i = 0; i < hvminfo->nr_vcpus; i++)
-        hvminfo->vcpu_online[i / 8] |= 1 << (i & 7);
+    memcpy(hvminfo->vcpu_online, b_info->avail_vcpus.map,
+           b_info->avail_vcpus.size);
 
     config->hvminfo = hvminfo;
 
-- 
2.7.4


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

[PATCH v4 14/15] pvh/acpi: Save ACPI registers for PVH guests

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v4:
* New patch

(This should eventually be moved in acpi.c)

 xen/arch/x86/hvm/pmtimer.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
index 5144928..a810f2b 100644
--- a/xen/arch/x86/hvm/pmtimer.c
+++ b/xen/arch/x86/hvm/pmtimer.c
@@ -257,7 +257,11 @@ static int acpi_save(struct domain *d, hvm_domain_context_t *h)
     int rc;
 
     if ( !has_vpm(d) )
+    {
+        if ( !has_acpi_dm_ff(d) )
+            return hvm_save_entry(ACPI, 0, h, acpi);
         return 0;
+    }
 
     spin_lock(&s->lock);
 
@@ -286,7 +290,11 @@ static int acpi_load(struct domain *d, hvm_domain_context_t *h)
     PMTState *s = &d->arch.hvm_domain.pl_time->vpmt;
 
     if ( !has_vpm(d) )
+    {
+        if ( !has_acpi_dm_ff(d) )
+            return hvm_load_entry(ACPI, h, acpi);
         return -ENODEV;
+    }
 
     spin_lock(&s->lock);
 
-- 
2.7.4


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

[PATCH v4 15/15] docs: Describe PVHv2's VCPU hotplug procedure

[Boris Ostrovsky]

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Tim Deegan <tim@xen.org>
---
Changes in v4:
* Updated text to reflect new interfaces.

 docs/misc/hvmlite.markdown | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/docs/misc/hvmlite.markdown b/docs/misc/hvmlite.markdown
index 898b8ee..71c6bc2 100644
--- a/docs/misc/hvmlite.markdown
+++ b/docs/misc/hvmlite.markdown
@@ -75,3 +75,16 @@ info structure that's passed at boot time (field rsdp_paddr).
 
 Description of paravirtualized devices will come from XenStore, just as it's
 done for HVM guests.
+
+## VCPU hotplug ##
+
+VCPU hotplug (e.g. 'xl vcpu-set <domain> <num_vcpus>') for PVHv2 guests
+follows ACPI model where change in domain's number of VCPUS (stored in
+domain.avail_vcpus) results in an SCI being sent to the guest. The guest
+then executes DSDT's PRSC method, updating MADT enable status for the
+affected VCPU.
+
+This is achieved by having the toolstack issue a write to ACPI's
+XEN_ACPI_CPU_MAP (thus updating the VCPU available map stored there),
+followed by a write to ACPI GPE0 status register, setting XEN_GPE0_CPUHP_BIT.
+The latter will cause an SCI to be generated.
-- 
2.7.4


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

Re: [PATCH v4 15/15] docs: Describe PVHv2's VCPU hotplug procedure

[Konrad Rzeszutek Wilk]

On Tue, Nov 29, 2016 at 10:33:22AM -0500, Boris Ostrovsky wrote:
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> ---
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Tim Deegan <tim@xen.org>
> ---
> Changes in v4:
> * Updated text to reflect new interfaces.
> 
>  docs/misc/hvmlite.markdown | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/docs/misc/hvmlite.markdown b/docs/misc/hvmlite.markdown
> index 898b8ee..71c6bc2 100644
> --- a/docs/misc/hvmlite.markdown
> +++ b/docs/misc/hvmlite.markdown
> @@ -75,3 +75,16 @@ info structure that's passed at boot time (field rsdp_paddr).
>  
>  Description of paravirtualized devices will come from XenStore, just as it's
>  done for HVM guests.
> +
> +## VCPU hotplug ##
> +
> +VCPU hotplug (e.g. 'xl vcpu-set <domain> <num_vcpus>') for PVHv2 guests
> +follows ACPI model where change in domain's number of VCPUS (stored in
> +domain.avail_vcpus) results in an SCI being sent to the guest. The guest
> +then executes DSDT's PRSC method, updating MADT enable status for the
> +affected VCPU.
> +
> +This is achieved by having the toolstack issue a write to ACPI's
> +XEN_ACPI_CPU_MAP (thus updating the VCPU available map stored there),
> +followed by a write to ACPI GPE0 status register, setting XEN_GPE0_CPUHP_BIT.
> +The latter will cause an SCI to be generated.
> -- 
> 2.7.4
> 

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

Re: [PATCH v4 00/15] PVH VCPU hotplug support

[Jan Beulich]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
> This series adds support for ACPI-based VCPU hotplug for unprivileged
> PVH guests.
> 
> Changes in v4:
> * Replaced XEN_DOMCTL_set_avail_vcpus with XEN_DOMCTL_acpi_access,
>   toolstack is expected to issue two ACPI "writes" to trigger the hotplug
> * Moved pm1a registers from PMTState to hvm_domain. gpe0 registers are
>   added there.
> * Both pm1a and gpe0 registers are now represented as uint16_t for status
>   and enable (and as result no new public macros for those registers' length
>   are needed). Ths is partly to avoid changes in HVM code (i.e. pmtimer)
>   and partly to simplify code. I don't expect larger registers will be
>   needed any time soon.
> * ACPI handling now lives in xen/arch/x86/hvm/acpi.c
> * Moved definition of XEN_ACPI_CPU_MAP/XEN_GPE0_CPUHP_BIT from ioreq.h
>   to public/arch-x86/xen.h. This is the best I could come up with but
>   ioreq is the wrong place, now that everything happens in acpi.c

Without having looked at the patch(es) - why not e.g.
public/arch-x86/hvm/acpi.h?

Jan


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

Re: [PATCH v4 00/15] PVH VCPU hotplug support

[Boris Ostrovsky]

On 11/29/2016 11:11 AM, Jan Beulich wrote:
>>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
>> This series adds support for ACPI-based VCPU hotplug for unprivileged
>> PVH guests.
>>
>> Changes in v4:
>> * Replaced XEN_DOMCTL_set_avail_vcpus with XEN_DOMCTL_acpi_access,
>>   toolstack is expected to issue two ACPI "writes" to trigger the hotplug
>> * Moved pm1a registers from PMTState to hvm_domain. gpe0 registers are
>>   added there.
>> * Both pm1a and gpe0 registers are now represented as uint16_t for status
>>   and enable (and as result no new public macros for those registers' length
>>   are needed). Ths is partly to avoid changes in HVM code (i.e. pmtimer)
>>   and partly to simplify code. I don't expect larger registers will be
>>   needed any time soon.
>> * ACPI handling now lives in xen/arch/x86/hvm/acpi.c
>> * Moved definition of XEN_ACPI_CPU_MAP/XEN_GPE0_CPUHP_BIT from ioreq.h
>>   to public/arch-x86/xen.h. This is the best I could come up with but
>>   ioreq is the wrong place, now that everything happens in acpi.c
> Without having looked at the patch(es) - why not e.g.
> public/arch-x86/hvm/acpi.h?

It's just 3 defines so I am not sure it's worth a whole new include file.

-boris

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

Re: [PATCH v4 00/15] PVH VCPU hotplug support

[Jan Beulich]

>>> On 29.11.16 at 17:40, <boris.ostrovsky@oracle.com> wrote:
> On 11/29/2016 11:11 AM, Jan Beulich wrote:
>>>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
>>> This series adds support for ACPI-based VCPU hotplug for unprivileged
>>> PVH guests.
>>>
>>> Changes in v4:
>>> * Replaced XEN_DOMCTL_set_avail_vcpus with XEN_DOMCTL_acpi_access,
>>>   toolstack is expected to issue two ACPI "writes" to trigger the hotplug
>>> * Moved pm1a registers from PMTState to hvm_domain. gpe0 registers are
>>>   added there.
>>> * Both pm1a and gpe0 registers are now represented as uint16_t for status
>>>   and enable (and as result no new public macros for those registers' length
>>>   are needed). Ths is partly to avoid changes in HVM code (i.e. pmtimer)
>>>   and partly to simplify code. I don't expect larger registers will be
>>>   needed any time soon.
>>> * ACPI handling now lives in xen/arch/x86/hvm/acpi.c
>>> * Moved definition of XEN_ACPI_CPU_MAP/XEN_GPE0_CPUHP_BIT from ioreq.h
>>>   to public/arch-x86/xen.h. This is the best I could come up with but
>>>   ioreq is the wrong place, now that everything happens in acpi.c
>> Without having looked at the patch(es) - why not e.g.
>> public/arch-x86/hvm/acpi.h?
> 
> It's just 3 defines so I am not sure it's worth a whole new include file.

Are you reasonably convinced that nothing else will want to be put
there soon? There were quite a few other ACPI additions in the
previous version of your series iirc, some or all of which may also
better fit here ...

Jan


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

Re: [PATCH v4 00/15] PVH VCPU hotplug support

[Boris Ostrovsky]

On 11/29/2016 11:43 AM, Jan Beulich wrote:
>>>> On 29.11.16 at 17:40, <boris.ostrovsky@oracle.com> wrote:
>> On 11/29/2016 11:11 AM, Jan Beulich wrote:
>>>>>> On 29.11.16 at 16:33, <boris.ostrovsky@oracle.com> wrote:
>>>> This series adds support for ACPI-based VCPU hotplug for unprivileged
>>>> PVH guests.
>>>>
>>>> Changes in v4:
>>>> * Replaced XEN_DOMCTL_set_avail_vcpus with XEN_DOMCTL_acpi_access,
>>>>   toolstack is expected to issue two ACPI "writes" to trigger the hotplug
>>>> * Moved pm1a registers from PMTState to hvm_domain. gpe0 registers are
>>>>   added there.
>>>> * Both pm1a and gpe0 registers are now represented as uint16_t for status
>>>>   and enable (and as result no new public macros for those registers' length
>>>>   are needed). Ths is partly to avoid changes in HVM code (i.e. pmtimer)
>>>>   and partly to simplify code. I don't expect larger registers will be
>>>>   needed any time soon.
>>>> * ACPI handling now lives in xen/arch/x86/hvm/acpi.c
>>>> * Moved definition of XEN_ACPI_CPU_MAP/XEN_GPE0_CPUHP_BIT from ioreq.h
>>>>   to public/arch-x86/xen.h. This is the best I could come up with but
>>>>   ioreq is the wrong place, now that everything happens in acpi.c
>>> Without having looked at the patch(es) - why not e.g.
>>> public/arch-x86/hvm/acpi.h?
>> It's just 3 defines so I am not sure it's worth a whole new include file.
> Are you reasonably convinced that nothing else will want to be put
> there soon? There were quite a few other ACPI additions in the
> previous version of your series iirc, some or all of which may also
> better fit here ...

These other things are gone now, that's what the 3rd bullet above refers
to. The only new public definitions are in patch 5. Of course, we may
expand libacpi in the future and new common things may be needed.

I also think at some point it may be worth replacing (at least
partially) tools/libacpi/acpi2_0.h with some files from
xen/include/acpi/ but that will require some non-trivial surgery.


-boris



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