* [PATCH v5 01/13] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-19 14:12 ` Jan Beulich
2016-12-16 23:18 ` [PATCH v5 02/13] acpi/x86: Define ACPI IO registers for PVH guests Boris Ostrovsky
` (11 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
These registers (pm1a specifically) are not all specific to pm timer
and are accessed by non-pmtimer code (for example, sleep/power button
emulation).
The public name for save state structure is kept as 'pmtimer' to avoid
code churn with the expected changes in migration code. hvm_hw_acpi
name is introduced for internal use but when migration code is updated
hvm_hw_pmtimer will be renamed to hvm_hw_acpi.
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 v5:
* Keep PMTIMER name for the save datastructure (i.e. hvm_hw_pmtimer)
* Replace tmr_val pointer werite with write_atomic()
* Code style cleanup
xen/arch/x86/hvm/pmtimer.c | 67 +++++++++++++++++++++++++---------------
xen/include/asm-x86/hvm/domain.h | 10 ++++++
xen/include/asm-x86/hvm/vpt.h | 1 -
3 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
index 99d1e86..b70c299 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;
+ write_atomic(&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(PMTIMER, 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(PMTIMER, 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(PMTIMER, 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..d55d180 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -102,6 +102,16 @@ struct hvm_domain {
struct hvm_vioapic *vioapic;
struct hvm_hw_stdvga stdvga;
+ /*
+ * hvm_hw_pmtimer is a publicly-visible name. We will defer renaming
+ * it to the more appropriate hvm_hw_acpi until the expected
+ * comprehensive rewrte of migration code, thus avoiding code churn
+ * in public header files.
+ * Internally, however, we will be using hvm_hw_acpi.
+ */
+#define hvm_hw_acpi hvm_hw_pmtimer
+ 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 */
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 01/13] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain
2016-12-16 23:18 ` [PATCH v5 01/13] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain Boris Ostrovsky
@ 2016-12-19 14:12 ` Jan Beulich
0 siblings, 0 replies; 44+ messages in thread
From: Jan Beulich @ 2016-12-19 14:12 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> 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).
>
> The public name for save state structure is kept as 'pmtimer' to avoid
> code churn with the expected changes in migration code. hvm_hw_acpi
> name is introduced for internal use but when migration code is updated
> hvm_hw_pmtimer will be renamed to hvm_hw_acpi.
>
> 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>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 02/13] acpi/x86: Define ACPI IO registers for PVH guests
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
2016-12-16 23:18 ` [PATCH v5 01/13] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-20 18:07 ` Julien Grall
2016-12-16 23:18 ` [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access Boris Ostrovsky
` (10 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
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>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v5:
* Renamed XEN_GPE0_CPUHP_BIT to XEN_ACPI_GPE0_CPUHP_BIT
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 639d21e..9421f3f 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(CONFIG_X86)
+#include <xen/arch-x86/xen.h>
#include <xen/hvm/hvm_info_table.h>
#elif defined(CONFIG_ARM_64)
#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_ACPI_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 1f6247d..608c936 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..12f719d 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_ACPI_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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 02/13] acpi/x86: Define ACPI IO registers for PVH guests
2016-12-16 23:18 ` [PATCH v5 02/13] acpi/x86: Define ACPI IO registers for PVH guests Boris Ostrovsky
@ 2016-12-20 18:07 ` Julien Grall
0 siblings, 0 replies; 44+ messages in thread
From: Julien Grall @ 2016-12-20 18:07 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel
Cc: Stefano Stabellini, wei.liu2, andrew.cooper3, ian.jackson,
jbeulich, roger.pau
Hello Boris,
This patch is breaking compilation of mk_dsdt on ARM64 (see below).
Boris can you please send a patch to fix this?
In the future, please make sure that mk_dsdt at least build for all the
targeted architectures.
mk_dsdt.c: In function 'main':
mk_dsdt.c:249:9: error: 'XEN_ACPI_CPU_MAP' undeclared (first use in this
function)
XEN_ACPI_CPU_MAP, XEN_ACPI_CPU_MAP_LEN);
^
mk_dsdt.c:63:25: note: in definition of macro 'stmt'
_stmt(n, f , ## a ); \
^
mk_dsdt.c:249:9: note: each undeclared identifier is reported only once
for each function it appears in
XEN_ACPI_CPU_MAP, XEN_ACPI_CPU_MAP_LEN);
^
mk_dsdt.c:63:25: note: in definition of macro 'stmt'
_stmt(n, f , ## a ); \
^
mk_dsdt.c:249:27: error: 'XEN_ACPI_CPU_MAP_LEN' undeclared (first use in
this function)
XEN_ACPI_CPU_MAP, XEN_ACPI_CPU_MAP_LEN);
^
mk_dsdt.c:63:25: note: in definition of macro 'stmt'
_stmt(n, f , ## a ); \
^
mk_dsdt.c:289:16: error: 'XEN_ACPI_GPE0_CPUHP_BIT' undeclared (first use
in this function)
XEN_ACPI_GPE0_CPUHP_BIT);
^
mk_dsdt.c:69:25: note: in definition of macro 'push_block'
_stmt(n, f , ## a ); \
^
Cheers,
On 17/12/2016 00:18, Boris Ostrovsky wrote:
> 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>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---
> Changes in v5:
> * Renamed XEN_GPE0_CPUHP_BIT to XEN_ACPI_GPE0_CPUHP_BIT
>
> 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 639d21e..9421f3f 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(CONFIG_X86)
> +#include <xen/arch-x86/xen.h>
> #include <xen/hvm/hvm_info_table.h>
> #elif defined(CONFIG_ARM_64)
> #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_ACPI_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 1f6247d..608c936 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..12f719d 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_ACPI_GPE0_CPUHP_BIT 2
> #endif
>
> #endif /* !__ASSEMBLY__ */
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
2016-12-16 23:18 ` [PATCH v5 01/13] x86/pmtimer: Move ACPI registers from PMTState to hvm_domain Boris Ostrovsky
2016-12-16 23:18 ` [PATCH v5 02/13] acpi/x86: Define ACPI IO registers for PVH guests Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-19 14:17 ` Jan Beulich
2016-12-16 23:18 ` [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses Boris Ostrovsky
` (9 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
Daniel De Graaf, roger.pau
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>
---
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
Changes in v5:
* Added forgotten in v4 xsm hooks
* Replaces gas_t with a xen_acpi_access_t
* Added structure padding
* Changed xen_domctl_acpi_access.val from uint8* to void*
* Constified hvm_acpi_domctl_access access pointer
tools/flask/policy/modules/dom0.te | 2 +-
tools/flask/policy/modules/xen.if | 4 ++--
xen/arch/x86/domctl.c | 9 +++++++++
xen/arch/x86/hvm/Makefile | 1 +
xen/arch/x86/hvm/acpi.c | 25 +++++++++++++++++++++++++
xen/include/asm-x86/hvm/domain.h | 4 ++++
xen/include/public/domctl.h | 25 +++++++++++++++++++++++++
xen/xsm/flask/hooks.c | 3 +++
xen/xsm/flask/policy/access_vectors | 2 ++
9 files changed, 72 insertions(+), 3 deletions(-)
create mode 100644 xen/arch/x86/hvm/acpi.c
diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index d0a4d91..475d446 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -39,7 +39,7 @@ allow dom0_t dom0_t:domain {
};
allow dom0_t dom0_t:domain2 {
set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo
- get_vnumainfo psr_cmt_op psr_cat_op
+ get_vnumainfo psr_cmt_op psr_cat_op acpi_access
};
allow dom0_t dom0_t:resource { add remove };
diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if
index eb646f5..59e4441 100644
--- a/tools/flask/policy/modules/xen.if
+++ b/tools/flask/policy/modules/xen.if
@@ -52,7 +52,7 @@ define(`create_domain_common', `
settime setdomainhandle getvcpucontext };
allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
- psr_cmt_op psr_cat_op soft_reset };
+ psr_cmt_op psr_cat_op soft_reset acpi_access };
allow $1 $2:security check_context;
allow $1 $2:shadow enable;
allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op updatemp };
@@ -85,7 +85,7 @@ define(`manage_domain', `
getaddrsize pause unpause trigger shutdown destroy
setaffinity setdomainmaxmem getscheduler resume
setpodtarget getpodtarget };
- allow $1 $2:domain2 set_vnumainfo;
+ allow $1 $2:domain2 { set_vnumainfo acpi_access };
')
# migrate_domain_out(priv, target)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index ab9ad39..00914f6 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1425,6 +1425,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.access,
+ 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..013b399
--- /dev/null
+++ b/xen/arch/x86/hvm/acpi.c
@@ -0,0 +1,25 @@
+/* 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,
+ const xen_acpi_access_t *access,
+ XEN_GUEST_HANDLE_PARAM(void) 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 d55d180..32880de 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -166,6 +166,10 @@ struct hvm_domain {
#define hap_enabled(d) ((d)->arch.hvm_domain.hap_enabled)
+int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
+ const xen_acpi_access_t *access,
+ XEN_GUEST_HANDLE_PARAM(void) arg);
+
#endif /* __ASM_X86_HVM_DOMAIN_H__ */
/*
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 177319d..ba9d367 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 access structure */
+typedef struct xen_acpi_access {
+#define XEN_ACPI_SYSTEM_MEMORY 0
+#define XEN_ACPI_SYSTEM_IO 1
+ uint8_t space_id; /* Address space */
+ uint8_t width; /* Access size (bytes) */
+ uint8_t pad[6];
+ uint64_aligned_t address; /* 64-bit address of register */
+} xen_acpi_access_t;
+
+struct xen_domctl_acpi_access {
+ xen_acpi_access_t access; /* IN: Register being accessed */
+
+#define XEN_DOMCTL_ACPI_READ 0
+#define XEN_DOMCTL_ACPI_WRITE 1
+ uint8_t rw; /* IN: Read or write */
+ uint8_t pad[7];
+
+ XEN_GUEST_HANDLE_64(void) 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;
};
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 040a251..c1ba42e 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -748,6 +748,9 @@ static int flask_domctl(struct domain *d, int cmd)
case XEN_DOMCTL_soft_reset:
return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__SOFT_RESET);
+ case XEN_DOMCTL_acpi_access:
+ return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__ACPI_ACCESS);
+
default:
return avc_unknown_permission("domctl", cmd);
}
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 92e6da9..e40258e 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -246,6 +246,8 @@ class domain2
mem_sharing
# XEN_DOMCTL_psr_cat_op
psr_cat_op
+# XEN_DOMCTL_acpi_access
+ acpi_access
}
# Similar to class domain, but primarily contains domctls related to HVM domains
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access
2016-12-16 23:18 ` [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access Boris Ostrovsky
@ 2016-12-19 14:17 ` Jan Beulich
2016-12-19 14:48 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-19 14:17 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: wei.liu2, andrew.cooper3, ian.jackson, xen-devel, Daniel De Graaf,
roger.pau
>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1425,6 +1425,15 @@ long arch_do_domctl(
> }
> break;
>
> + case XEN_DOMCTL_acpi_access:
> + if ( !is_hvm_domain(d) )
> + ret = -EINVAL;
I think it would be better to use some other, less frequently used
error code here.
> --- /dev/null
> +++ b/xen/arch/x86/hvm/acpi.c
> @@ -0,0 +1,25 @@
> +/* 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,
Wouldn't "rw" better be bool internally?
> --- 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 access structure */
> +typedef struct xen_acpi_access {
> +#define XEN_ACPI_SYSTEM_MEMORY 0
> +#define XEN_ACPI_SYSTEM_IO 1
> + uint8_t space_id; /* Address space */
> + uint8_t width; /* Access size (bytes) */
> + uint8_t pad[6];
> + uint64_aligned_t address; /* 64-bit address of register */
> +} xen_acpi_access_t;
> +
> +struct xen_domctl_acpi_access {
> + xen_acpi_access_t access; /* IN: Register being accessed */
> +
> +#define XEN_DOMCTL_ACPI_READ 0
> +#define XEN_DOMCTL_ACPI_WRITE 1
> + uint8_t rw; /* IN: Read or write */
> + uint8_t pad[7];
> +
> + XEN_GUEST_HANDLE_64(void) val; /* IN/OUT: data */
> +};
> +typedef struct xen_domctl_acpi_access xen_domctl_acpi_access_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_acpi_access_t);
Do you expect to use xen_acpi_access anywhere else? If not,
the overall amount of padding needed could be reduced by
folding both structures.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access
2016-12-19 14:17 ` Jan Beulich
@ 2016-12-19 14:48 ` Boris Ostrovsky
2016-12-19 14:53 ` Jan Beulich
0 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-19 14:48 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, andrew.cooper3, ian.jackson, xen-devel, Daniel De Graaf,
roger.pau
On 12/19/2016 09:17 AM, Jan Beulich wrote:
>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -1425,6 +1425,15 @@ long arch_do_domctl(
>> }
>> break;
>>
>> + case XEN_DOMCTL_acpi_access:
>> + if ( !is_hvm_domain(d) )
>> + ret = -EINVAL;
> I think it would be better to use some other, less frequently used
> error code here.
ENODEV? (Because there is no ACPI "device" for PV guests)
>
>> --- /dev/null
>> +++ b/xen/arch/x86/hvm/acpi.c
>> @@ -0,0 +1,25 @@
>> +/* 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,
> Wouldn't "rw" better be bool internally?
I will drop it altogether since as you pointed out below there is no
reason to keep xen_acpi_access_t as a separate structure. I'll pass
whole xen_domctl_acpi_access_t here.
(And I noticed that XEN_DOMCTL_ACPI_WRITE crept into acpi_common_*()
code in patch 5 so I will want to fix dir/rw there)
-boris
>
>> --- 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 access structure */
>> +typedef struct xen_acpi_access {
>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>> +#define XEN_ACPI_SYSTEM_IO 1
>> + uint8_t space_id; /* Address space */
>> + uint8_t width; /* Access size (bytes) */
>> + uint8_t pad[6];
>> + uint64_aligned_t address; /* 64-bit address of register */
>> +} xen_acpi_access_t;
>> +
>> +struct xen_domctl_acpi_access {
>> + xen_acpi_access_t access; /* IN: Register being accessed */
>> +
>> +#define XEN_DOMCTL_ACPI_READ 0
>> +#define XEN_DOMCTL_ACPI_WRITE 1
>> + uint8_t rw; /* IN: Read or write */
>> + uint8_t pad[7];
>> +
>> + XEN_GUEST_HANDLE_64(void) val; /* IN/OUT: data */
>> +};
>> +typedef struct xen_domctl_acpi_access xen_domctl_acpi_access_t;
>> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_acpi_access_t);
> Do you expect to use xen_acpi_access anywhere else? If not,
> the overall amount of padding needed could be reduced by
> folding both structures.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access
2016-12-19 14:48 ` Boris Ostrovsky
@ 2016-12-19 14:53 ` Jan Beulich
0 siblings, 0 replies; 44+ messages in thread
From: Jan Beulich @ 2016-12-19 14:53 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: wei.liu2, andrew.cooper3, ian.jackson, xen-devel, Daniel De Graaf,
roger.pau
>>> On 19.12.16 at 15:48, <boris.ostrovsky@oracle.com> wrote:
> On 12/19/2016 09:17 AM, Jan Beulich wrote:
>>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>> --- a/xen/arch/x86/domctl.c
>>> +++ b/xen/arch/x86/domctl.c
>>> @@ -1425,6 +1425,15 @@ long arch_do_domctl(
>>> }
>>> break;
>>>
>>> + case XEN_DOMCTL_acpi_access:
>>> + if ( !is_hvm_domain(d) )
>>> + ret = -EINVAL;
>> I think it would be better to use some other, less frequently used
>> error code here.
>
> ENODEV? (Because there is no ACPI "device" for PV guests)
Fine with me.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (2 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-20 11:24 ` Jan Beulich
2016-12-16 23:18 ` [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests Boris Ostrovsky
` (8 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
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 v5:
* Split VCPU map and ACPI register access routines
* Added compat restore routine
xen/arch/x86/hvm/acpi.c | 31 +++++++++++++++++++++++++++++++
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 | 31 +++++++++++++++++++++++++++++++
xen/include/public/arch-x86/xen.h | 4 +++-
6 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index 013b399..9b2885e 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,
const xen_acpi_access_t *access,
@@ -14,6 +15,36 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
return -ENOSYS;
}
+static int acpi_guest_access(int dir, unsigned int port,
+ unsigned int bytes, uint32_t *val)
+{
+ return X86EMUL_UNHANDLEABLE;
+}
+
+static int acpi_cpumap_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_cpumap_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 2b3977a..8d2dfb8 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 39cc658..6470c53 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -426,6 +426,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 32880de..58fba33 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -166,6 +166,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 *d, uint8_t rw,
const xen_acpi_access_t *access,
XEN_GUEST_HANDLE_PARAM(void) arg);
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h
index 8d73b51..b1d560f 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
/*
* PM timer
*/
+#if __XEN_INTERFACE_VERSION__ >= 0x00040800
+struct hvm_hw_pmtimer {
+ 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 */
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
+ uint16_t gpe0_sts;
+ uint16_t gpe0_en;
+#endif
+};
+
+struct hvm_hw_pmtimer_compat {
+ uint32_t tmr_val;
+ uint16_t pm1a_sts;
+ uint16_t pm1a_en;
+};
+
+static inline int _hvm_hw_fix_pmtimer(void *h, uint32_t size)
+{
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
+ struct hvm_hw_pmtimer *acpi = (struct hvm_hw_pmtimer *)h;
+
+ if ( size == sizeof(struct hvm_hw_pmtimer_compat) )
+ acpi->gpe0_sts = acpi->gpe0_en = 0;
+#endif
+ return 0;
+}
+DECLARE_HVM_SAVE_TYPE_COMPAT(PMTIMER, 13, struct hvm_hw_pmtimer, \
+ struct hvm_hw_pmtimer_compat, _hvm_hw_fix_pmtimer);
+#else /* __XEN_INTERFACE_VERSION__ */
struct hvm_hw_pmtimer {
uint32_t tmr_val; /* PM_TMR_BLK.TMR_VAL: 32bit free-running counter */
uint16_t pm1a_sts; /* PM1a_EVT_BLK.PM1a_STS: status register */
@@ -535,6 +565,7 @@ struct hvm_hw_pmtimer {
};
DECLARE_HVM_SAVE_TYPE(PMTIMER, 13, struct hvm_hw_pmtimer);
+#endif /* __XEN_INTERFACE_VERSION__ */
/*
* MTRR MSRs
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 12f719d..2565acd 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-16 23:18 ` [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses Boris Ostrovsky
@ 2016-12-20 11:24 ` Jan Beulich
2016-12-20 14:03 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 11:24 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/include/public/arch-x86/hvm/save.h
> +++ b/xen/include/public/arch-x86/hvm/save.h
> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
> /*
> * PM timer
> */
> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
> +struct hvm_hw_pmtimer {
> + 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 */
> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> + uint16_t gpe0_sts;
> + uint16_t gpe0_en;
> +#endif
Why inside another #ifdef? There's no other example in this file
which might have suggested to you that it needs doing this way.
In fact there are also no pre-existing uses of
__XEN_INTERFACE_VERSION__ in this header, so I also don't see
why you added one (and then with a slightly off value check).
If anything the _whole_ header would need to become Xen/tools
only.
> +static inline int _hvm_hw_fix_pmtimer(void *h, uint32_t size)
> +{
> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> + struct hvm_hw_pmtimer *acpi = (struct hvm_hw_pmtimer *)h;
> +
> + if ( size == sizeof(struct hvm_hw_pmtimer_compat) )
> + acpi->gpe0_sts = acpi->gpe0_en = 0;
> +#endif
Same here.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-20 11:24 ` Jan Beulich
@ 2016-12-20 14:03 ` Boris Ostrovsky
2016-12-20 14:10 ` Jan Beulich
0 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 14:03 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
On 12/20/2016 06:24 AM, Jan Beulich wrote:
>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/include/public/arch-x86/hvm/save.h
>> +++ b/xen/include/public/arch-x86/hvm/save.h
>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>> /*
>> * PM timer
>> */
>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>> +struct hvm_hw_pmtimer {
>> + 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 */
>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>> + uint16_t gpe0_sts;
>> + uint16_t gpe0_en;
>> +#endif
> Why inside another #ifdef? There's no other example in this file
> which might have suggested to you that it needs doing this way.
> In fact there are also no pre-existing uses of
> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
> why you added one (and then with a slightly off value check).
Don't we want users of old interface to continue using original
definition of hvm_hw_timer? And not to expose them to the fix routine below?
-boris
>
> If anything the _whole_ header would need to become Xen/tools
> only.
>
>> +static inline int _hvm_hw_fix_pmtimer(void *h, uint32_t size)
>> +{
>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>> + struct hvm_hw_pmtimer *acpi = (struct hvm_hw_pmtimer *)h;
>> +
>> + if ( size == sizeof(struct hvm_hw_pmtimer_compat) )
>> + acpi->gpe0_sts = acpi->gpe0_en = 0;
>> +#endif
> Same here.
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-20 14:03 ` Boris Ostrovsky
@ 2016-12-20 14:10 ` Jan Beulich
2016-12-20 14:16 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 14:10 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 20.12.16 at 15:03, <boris.ostrovsky@oracle.com> wrote:
> On 12/20/2016 06:24 AM, Jan Beulich wrote:
>>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>> /*
>>> * PM timer
>>> */
>>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>>> +struct hvm_hw_pmtimer {
>>> + 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 */
>>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>>> + uint16_t gpe0_sts;
>>> + uint16_t gpe0_en;
>>> +#endif
>> Why inside another #ifdef? There's no other example in this file
>> which might have suggested to you that it needs doing this way.
>> In fact there are also no pre-existing uses of
>> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
>> why you added one (and then with a slightly off value check).
>
> Don't we want users of old interface to continue using original
> definition of hvm_hw_timer? And not to expose them to the fix routine below?
There shouldn't be any such old users, because of ...
>> If anything the _whole_ header would need to become Xen/tools
>> only.
... this.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-20 14:10 ` Jan Beulich
@ 2016-12-20 14:16 ` Boris Ostrovsky
2016-12-20 14:45 ` Jan Beulich
0 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 14:16 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
On 12/20/2016 09:10 AM, Jan Beulich wrote:
>>>> On 20.12.16 at 15:03, <boris.ostrovsky@oracle.com> wrote:
>> On 12/20/2016 06:24 AM, Jan Beulich wrote:
>>>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>>> /*
>>>> * PM timer
>>>> */
>>>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>>>> +struct hvm_hw_pmtimer {
>>>> + 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 */
>>>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>>>> + uint16_t gpe0_sts;
>>>> + uint16_t gpe0_en;
>>>> +#endif
>>> Why inside another #ifdef? There's no other example in this file
>>> which might have suggested to you that it needs doing this way.
>>> In fact there are also no pre-existing uses of
>>> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
>>> why you added one (and then with a slightly off value check).
>> Don't we want users of old interface to continue using original
>> definition of hvm_hw_timer? And not to expose them to the fix routine below?
> There shouldn't be any such old users, because of ...
>
>>> If anything the _whole_ header would need to become Xen/tools
>>> only.
> ... this.
Is this file is not supposed to be used by anyone outside of the Xen tree?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-20 14:16 ` Boris Ostrovsky
@ 2016-12-20 14:45 ` Jan Beulich
2016-12-20 14:55 ` Andrew Cooper
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 14:45 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 20.12.16 at 15:16, <boris.ostrovsky@oracle.com> wrote:
> On 12/20/2016 09:10 AM, Jan Beulich wrote:
>>>>> On 20.12.16 at 15:03, <boris.ostrovsky@oracle.com> wrote:
>>> On 12/20/2016 06:24 AM, Jan Beulich wrote:
>>>>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>>>> /*
>>>>> * PM timer
>>>>> */
>>>>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>>>>> +struct hvm_hw_pmtimer {
>>>>> + 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 */
>>>>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>>>>> + uint16_t gpe0_sts;
>>>>> + uint16_t gpe0_en;
>>>>> +#endif
>>>> Why inside another #ifdef? There's no other example in this file
>>>> which might have suggested to you that it needs doing this way.
>>>> In fact there are also no pre-existing uses of
>>>> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
>>>> why you added one (and then with a slightly off value check).
>>> Don't we want users of old interface to continue using original
>>> definition of hvm_hw_timer? And not to expose them to the fix routine below?
>> There shouldn't be any such old users, because of ...
>>
>>>> If anything the _whole_ header would need to become Xen/tools
>>>> only.
>> ... this.
>
> Is this file is not supposed to be used by anyone outside of the Xen tree?
I don't think so, no. In any event - prior additions did not do
any precautions to guard possible foreign consumers. Maybe
Andrew has an opinion here ...
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-20 14:45 ` Jan Beulich
@ 2016-12-20 14:55 ` Andrew Cooper
2016-12-20 15:31 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Cooper @ 2016-12-20 14:55 UTC (permalink / raw)
To: Jan Beulich, Boris Ostrovsky; +Cc: ian.jackson, xen-devel, wei.liu2, roger.pau
On 20/12/2016 14:45, Jan Beulich wrote:
>>>> On 20.12.16 at 15:16, <boris.ostrovsky@oracle.com> wrote:
>> On 12/20/2016 09:10 AM, Jan Beulich wrote:
>>>>>> On 20.12.16 at 15:03, <boris.ostrovsky@oracle.com> wrote:
>>>> On 12/20/2016 06:24 AM, Jan Beulich wrote:
>>>>>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>>>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>>>>> /*
>>>>>> * PM timer
>>>>>> */
>>>>>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>>>>>> +struct hvm_hw_pmtimer {
>>>>>> + 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 */
>>>>>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>>>>>> + uint16_t gpe0_sts;
>>>>>> + uint16_t gpe0_en;
>>>>>> +#endif
>>>>> Why inside another #ifdef? There's no other example in this file
>>>>> which might have suggested to you that it needs doing this way.
>>>>> In fact there are also no pre-existing uses of
>>>>> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
>>>>> why you added one (and then with a slightly off value check).
>>>> Don't we want users of old interface to continue using original
>>>> definition of hvm_hw_timer? And not to expose them to the fix routine below?
>>> There shouldn't be any such old users, because of ...
>>>
>>>>> If anything the _whole_ header would need to become Xen/tools
>>>>> only.
>>> ... this.
>> Is this file is not supposed to be used by anyone outside of the Xen tree?
> I don't think so, no. In any event - prior additions did not do
> any precautions to guard possible foreign consumers. Maybe
> Andrew has an opinion here ...
Our policies along these lines are a mess. We really should separate
the guest ABI/API from the toolstack ABI/API, and by this, I mean having
separate directory structures.
In the meantime, all of the content in this file is only useful to
entities which can make DOMCTLs to start with, so the entire file should
be restricted to Xen/tools only.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses
2016-12-20 14:55 ` Andrew Cooper
@ 2016-12-20 15:31 ` Boris Ostrovsky
0 siblings, 0 replies; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 15:31 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich; +Cc: ian.jackson, xen-devel, wei.liu2, roger.pau
On 12/20/2016 09:55 AM, Andrew Cooper wrote:
>>> Is this file is not supposed to be used by anyone outside of the Xen tree?
>> I don't think so, no. In any event - prior additions did not do
>> any precautions to guard possible foreign consumers. Maybe
>> Andrew has an opinion here ...
> Our policies along these lines are a mess. We really should separate
> the guest ABI/API from the toolstack ABI/API, and by this, I mean having
> separate directory structures.
>
> In the meantime, all of the content in this file is only useful to
> entities which can make DOMCTLs to start with, so the entire file should
> be restricted to Xen/tools only.
I can add that but it should be done as a separate patch as it has
nothing to do with what the series is trying to provide.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (3 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-20 11:50 ` Jan Beulich
2016-12-16 23:18 ` [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl Boris Ostrovsky
` (7 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
Subsequent domctl access to ACPI registers and VCPU map will use the
same code. We create acpi_[cpumap_]access_common() routines in anticipation
of these changes.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v5:
* Code movement due to changes in patch 4
* Added fallthough switch statemnt comments
* Free d->avail_vcpus
xen/arch/x86/hvm/acpi.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++-
xen/common/domain.c | 1 +
xen/common/domctl.c | 5 ++
xen/include/xen/sched.h | 3 ++
4 files changed, 134 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index 9b2885e..b2299a4 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_cpumap_access_common(struct domain *d,
+ int dir, unsigned int port,
+ unsigned int bytes, uint32_t *val)
+{
+ unsigned int first_byte = port - XEN_ACPI_CPU_MAP;
+
+ BUILD_BUG_ON(XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN
+ >= ACPI_GPE0_BLK_ADDRESS_V1);
+
+ if ( dir == XEN_DOMCTL_ACPI_READ )
+ {
+ uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
+
+ /*
+ * 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
+ /* Guests do not write CPU map */
+ return X86EMUL_UNHANDLEABLE;
+
+ return X86EMUL_OKAY;
+}
+
+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_ACPI_GPE0_CPUHP_BIT;
+ static const uint16_t gpe0_en_mask = 1U << XEN_ACPI_GPE0_CPUHP_BIT;
+
+ 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;
+
+ default:
+ return X86EMUL_UNHANDLEABLE;
+ }
+
+ if ( dir == XEN_DOMCTL_ACPI_READ )
+ {
+ uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
+ uint32_t data = (((uint32_t)*en) << 16) | *sts;
+
+ data >>= 8 * (port & 3);
+ *val = (*val & mask) | (data & ~mask);
+ }
+ else
+ {
+ 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;
+ /* fallthrough */
+ case 1:
+ *sts &= ~((v & 0xff) << 8);
+ *sts &= *mask_sts;
+ if ( !--bytes )
+ break;
+ v >>= 8;
+ /* fallthrough */
+ case 2:
+ *en = ((*en & 0xff00) | (v & 0xff)) & *mask_en;
+ if ( !--bytes )
+ break;
+ v >>= 8;
+ /* fallthrough */
+ case 3:
+ *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
+ break;
+ }
+ }
+
+ return X86EMUL_OKAY;
+}
+
+
int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
const xen_acpi_access_t *access,
XEN_GUEST_HANDLE_PARAM(void) arg)
@@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
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);
}
static int acpi_cpumap_guest_access(int dir, unsigned int port,
unsigned int bytes, uint32_t *val)
{
- return X86EMUL_UNHANDLEABLE;
+ return acpi_cpumap_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/domain.c b/xen/common/domain.c
index 3abaca9..cb8df09 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -847,6 +847,7 @@ static void complete_domain_destroy(struct rcu_head *head)
xsm_free_security_domain(d);
free_cpumask_var(d->domain_dirty_cpumask);
xfree(d->vcpu);
+ xfree(d->avail_vcpus);
free_domain_struct(d);
send_global_virq(VIRQ_DOM_EXC);
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 063efe6..bee190f 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-16 23:18 ` [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests Boris Ostrovsky
@ 2016-12-20 11:50 ` Jan Beulich
2016-12-20 14:35 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 11:50 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> +static int acpi_cpumap_access_common(struct domain *d,
> + int dir, unsigned int port,
> + unsigned int bytes, uint32_t *val)
> +{
> + unsigned int first_byte = port - XEN_ACPI_CPU_MAP;
> +
> + BUILD_BUG_ON(XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN
> + >= ACPI_GPE0_BLK_ADDRESS_V1);
Just > afaict.
> +
> + if ( dir == XEN_DOMCTL_ACPI_READ )
> + {
> + uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
> +
> + /*
> + * 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
> + /* Guests do not write CPU map */
> + return X86EMUL_UNHANDLEABLE;
Perhaps make this an early bail, reducing overall indentation?
> +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_ACPI_GPE0_CPUHP_BIT;
> + static const uint16_t gpe0_en_mask = 1U << XEN_ACPI_GPE0_CPUHP_BIT;
> +
> + 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;
> +
> + default:
> + return X86EMUL_UNHANDLEABLE;
> + }
> +
> + if ( dir == XEN_DOMCTL_ACPI_READ )
> + {
> + uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
> + uint32_t data = (((uint32_t)*en) << 16) | *sts;
There's one pair of pointless parentheses around the cast
expression here.
> +
> + data >>= 8 * (port & 3);
> + *val = (*val & mask) | (data & ~mask);
> + }
> + else
> + {
> + uint32_t v = *val;
> +
> + /* Status register is write-1-to-clear by guests */
> + switch ( port & 3 )
> + {
> + case 0:
> + *sts &= ~(v & 0xff);
> + *sts &= *mask_sts;
I can understand the first &=, but why would a read have this second
(side) effect? I could see some sort of need for such only when you
were setting any flags.
> + if ( !--bytes )
> + break;
> + v >>= 8;
> + /* fallthrough */
> + case 1:
> + *sts &= ~((v & 0xff) << 8);
> + *sts &= *mask_sts;
> + if ( !--bytes )
> + break;
> + v >>= 8;
> + /* fallthrough */
> + case 2:
> + *en = ((*en & 0xff00) | (v & 0xff)) & *mask_en;
> + if ( !--bytes )
> + break;
> + v >>= 8;
> + /* fallthrough */
> + case 3:
> + *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
> + break;
> + }
> + }
> +
> + return X86EMUL_OKAY;
> +}
> +
> +
> int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
No double blank lines please.
> @@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
> 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);
> }
I don't think this one is meant to be used by the domctl, so I don't see
why you need a helper here. That would also eliminate the seemingly
unnecessary use of XEN_DOMCTL_* here (I think you already had said
this was an oversight in an earlier reply).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-20 11:50 ` Jan Beulich
@ 2016-12-20 14:35 ` Boris Ostrovsky
2016-12-20 14:47 ` Jan Beulich
0 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 14:35 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
On 12/20/2016 06:50 AM, Jan Beulich wrote:
>
>> +
>> + if ( dir == XEN_DOMCTL_ACPI_READ )
>> + {
>> + uint32_t mask = (bytes < 4) ? ~0U << (bytes * 8) : 0;
>> + uint32_t data = (((uint32_t)*en) << 16) | *sts;
> There's one pair of pointless parentheses around the cast
> expression here.
>
>> +
>> + data >>= 8 * (port & 3);
>> + *val = (*val & mask) | (data & ~mask);
>> + }
>> + else
>> + {
>> + uint32_t v = *val;
>> +
>> + /* Status register is write-1-to-clear by guests */
>> + switch ( port & 3 )
>> + {
>> + case 0:
>> + *sts &= ~(v & 0xff);
>> + *sts &= *mask_sts;
> I can understand the first &=, but why would a read have this second
> (side) effect? I could see some sort of need for such only when you
> were setting any flags.
This is a write, not a read.
>
>> @@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
>> 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);
>> }
> I don't think this one is meant to be used by the domctl, so I don't see
> why you need a helper here. That would also eliminate the seemingly
> unnecessary use of XEN_DOMCTL_* here (I think you already had said
> this was an oversight in an earlier reply).
Yes, XEN_DOMCTL_*is out of place here but I'd prefer to keep the helper
to isolate the sense of 'dir' as is used by portio handling
infrastructure (i.e. IOREQ_READ/WRITE) from how it is defined in the
acpi code here. Especially if, as mentioned in another thread, I use
bool is_write in common routines.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-20 14:35 ` Boris Ostrovsky
@ 2016-12-20 14:47 ` Jan Beulich
2016-12-20 15:29 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 14:47 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 20.12.16 at 15:35, <boris.ostrovsky@oracle.com> wrote:
> On 12/20/2016 06:50 AM, Jan Beulich wrote:
>>> + else
>>> + {
>>> + uint32_t v = *val;
>>> +
>>> + /* Status register is write-1-to-clear by guests */
>>> + switch ( port & 3 )
>>> + {
>>> + case 0:
>>> + *sts &= ~(v & 0xff);
>>> + *sts &= *mask_sts;
>> I can understand the first &=, but why would a read have this second
>> (side) effect? I could see some sort of need for such only when you
>> were setting any flags.
>
> This is a write, not a read.
Oh, right. But the question remains about that unexpected side
effect.
>>> @@ -18,13 +135,19 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
>>> 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);
>>> }
>> I don't think this one is meant to be used by the domctl, so I don't see
>> why you need a helper here. That would also eliminate the seemingly
>> unnecessary use of XEN_DOMCTL_* here (I think you already had said
>> this was an oversight in an earlier reply).
>
> Yes, XEN_DOMCTL_*is out of place here but I'd prefer to keep the helper
> to isolate the sense of 'dir' as is used by portio handling
> infrastructure (i.e. IOREQ_READ/WRITE) from how it is defined in the
> acpi code here. Especially if, as mentioned in another thread, I use
> bool is_write in common routines.
Well, if the helper is needed for the domctl, then this is at least
acceptable (albeit personally I'd prefer the domctl handler to
do the translation, using the IOREQ_* values internally. If the
helper is not needed for domctl, then XEN_DOMCTL_* shouldn't
be passed here.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-20 14:47 ` Jan Beulich
@ 2016-12-20 15:29 ` Boris Ostrovsky
2016-12-20 15:41 ` Jan Beulich
0 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 15:29 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
On 12/20/2016 09:47 AM, Jan Beulich wrote:
>>>> On 20.12.16 at 15:35, <boris.ostrovsky@oracle.com> wrote:
>> On 12/20/2016 06:50 AM, Jan Beulich wrote:
>>>> + else
>>>> + {
>>>> + uint32_t v = *val;
>>>> +
>>>> + /* Status register is write-1-to-clear by guests */
>>>> + switch ( port & 3 )
>>>> + {
>>>> + case 0:
>>>> + *sts &= ~(v & 0xff);
>>>> + *sts &= *mask_sts;
>>> I can understand the first &=, but why would a read have this second
>>> (side) effect? I could see some sort of need for such only when you
>>> were setting any flags.
>> This is a write, not a read.
> Oh, right. But the question remains about that unexpected side
> effect.
It indeed doesn't do anything for the case of guest access. It is
guarding against setting unauthorized bits by domctl (introduced by the
next patch). And I can move it into that case.
However, as discussed in the thread about 06/13 patch, we may currently
not need to provide domctl access to anything but VCPU map. So the
question is whether domctl interface to GPE0/PM1a is needed at all. I
think it's a useful interface with very little extra code required and
the toolstack can use it, for example, to inject events into guests. But
I don't have a specific use case.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-20 15:29 ` Boris Ostrovsky
@ 2016-12-20 15:41 ` Jan Beulich
2016-12-20 16:46 ` Andrew Cooper
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 15:41 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 20.12.16 at 16:29, <boris.ostrovsky@oracle.com> wrote:
> On 12/20/2016 09:47 AM, Jan Beulich wrote:
>>>>> On 20.12.16 at 15:35, <boris.ostrovsky@oracle.com> wrote:
>>> On 12/20/2016 06:50 AM, Jan Beulich wrote:
>>>>> + else
>>>>> + {
>>>>> + uint32_t v = *val;
>>>>> +
>>>>> + /* Status register is write-1-to-clear by guests */
>>>>> + switch ( port & 3 )
>>>>> + {
>>>>> + case 0:
>>>>> + *sts &= ~(v & 0xff);
>>>>> + *sts &= *mask_sts;
>>>> I can understand the first &=, but why would a read have this second
>>>> (side) effect? I could see some sort of need for such only when you
>>>> were setting any flags.
>>> This is a write, not a read.
>> Oh, right. But the question remains about that unexpected side
>> effect.
>
> It indeed doesn't do anything for the case of guest access. It is
> guarding against setting unauthorized bits by domctl (introduced by the
> next patch). And I can move it into that case.
>
> However, as discussed in the thread about 06/13 patch, we may currently
> not need to provide domctl access to anything but VCPU map. So the
> question is whether domctl interface to GPE0/PM1a is needed at all. I
> think it's a useful interface with very little extra code required and
> the toolstack can use it, for example, to inject events into guests. But
> I don't have a specific use case.
To be honest, I'd keep out anything we don't really need.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-20 15:41 ` Jan Beulich
@ 2016-12-20 16:46 ` Andrew Cooper
2016-12-20 16:51 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Andrew Cooper @ 2016-12-20 16:46 UTC (permalink / raw)
To: Jan Beulich, Boris Ostrovsky; +Cc: ian.jackson, xen-devel, wei.liu2, roger.pau
On 20/12/2016 15:41, Jan Beulich wrote:
>>>> On 20.12.16 at 16:29, <boris.ostrovsky@oracle.com> wrote:
>> On 12/20/2016 09:47 AM, Jan Beulich wrote:
>>>>>> On 20.12.16 at 15:35, <boris.ostrovsky@oracle.com> wrote:
>>>> On 12/20/2016 06:50 AM, Jan Beulich wrote:
>>>>>> + else
>>>>>> + {
>>>>>> + uint32_t v = *val;
>>>>>> +
>>>>>> + /* Status register is write-1-to-clear by guests */
>>>>>> + switch ( port & 3 )
>>>>>> + {
>>>>>> + case 0:
>>>>>> + *sts &= ~(v & 0xff);
>>>>>> + *sts &= *mask_sts;
>>>>> I can understand the first &=, but why would a read have this second
>>>>> (side) effect? I could see some sort of need for such only when you
>>>>> were setting any flags.
>>>> This is a write, not a read.
>>> Oh, right. But the question remains about that unexpected side
>>> effect.
>> It indeed doesn't do anything for the case of guest access. It is
>> guarding against setting unauthorized bits by domctl (introduced by the
>> next patch). And I can move it into that case.
>>
>> However, as discussed in the thread about 06/13 patch, we may currently
>> not need to provide domctl access to anything but VCPU map. So the
>> question is whether domctl interface to GPE0/PM1a is needed at all. I
>> think it's a useful interface with very little extra code required and
>> the toolstack can use it, for example, to inject events into guests. But
>> I don't have a specific use case.
> To be honest, I'd keep out anything we don't really need.
I have two specific use-cases. 1) DIMM Hotplug, and 2) Windows
tablet/desktop mode switch, both of which I think will just require a
toolstack entity to be able to raise an SCI.
However, I am happy for you to ignore these case for now (as they
haven't yet been fully designed and thought through), on the assumption
that if we do need to add anything, it will happily sit alongside the
VCPU code.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests
2016-12-20 16:46 ` Andrew Cooper
@ 2016-12-20 16:51 ` Boris Ostrovsky
0 siblings, 0 replies; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 16:51 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich; +Cc: ian.jackson, xen-devel, wei.liu2, roger.pau
On 12/20/2016 11:46 AM, Andrew Cooper wrote:
> On 20/12/2016 15:41, Jan Beulich wrote:
>>>>> On 20.12.16 at 16:29, <boris.ostrovsky@oracle.com> wrote:
>>> On 12/20/2016 09:47 AM, Jan Beulich wrote:
>>>>>>> On 20.12.16 at 15:35, <boris.ostrovsky@oracle.com> wrote:
>>>>> On 12/20/2016 06:50 AM, Jan Beulich wrote:
>>>>>>> + else
>>>>>>> + {
>>>>>>> + uint32_t v = *val;
>>>>>>> +
>>>>>>> + /* Status register is write-1-to-clear by guests */
>>>>>>> + switch ( port & 3 )
>>>>>>> + {
>>>>>>> + case 0:
>>>>>>> + *sts &= ~(v & 0xff);
>>>>>>> + *sts &= *mask_sts;
>>>>>> I can understand the first &=, but why would a read have this second
>>>>>> (side) effect? I could see some sort of need for such only when you
>>>>>> were setting any flags.
>>>>> This is a write, not a read.
>>>> Oh, right. But the question remains about that unexpected side
>>>> effect.
>>> It indeed doesn't do anything for the case of guest access. It is
>>> guarding against setting unauthorized bits by domctl (introduced by the
>>> next patch). And I can move it into that case.
>>>
>>> However, as discussed in the thread about 06/13 patch, we may currently
>>> not need to provide domctl access to anything but VCPU map. So the
>>> question is whether domctl interface to GPE0/PM1a is needed at all. I
>>> think it's a useful interface with very little extra code required and
>>> the toolstack can use it, for example, to inject events into guests. But
>>> I don't have a specific use case.
>> To be honest, I'd keep out anything we don't really need.
> I have two specific use-cases. 1) DIMM Hotplug, and 2) Windows
> tablet/desktop mode switch, both of which I think will just require a
> toolstack entity to be able to raise an SCI.
And I just found these two: XEN_DOMCTL_SENDTRIGGER_POWER and
XEN_DOMCTL_SENDTRIGGER_POWER.
When we switch HVM to new interface these can be re-implemented on top
of ACPI access.
-boris
>
> However, I am happy for you to ignore these case for now (as they
> haven't yet been fully designed and thought through), on the assumption
> that if we do need to add anything, it will happily sit alongside the
> VCPU code.
>
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (4 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 05/13] pvh/acpi: Handle ACPI accesses for PVH guests Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-20 13:24 ` Jan Beulich
2016-12-16 23:18 ` [PATCH v5 07/13] events/x86: Define SCI virtual interrupt Boris Ostrovsky
` (6 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
Changes in v5:
* Code movement due to changes in patch 4
xen/arch/x86/hvm/acpi.c | 64 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 55 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index b2299a4..044699d 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_cpumap_access_common(struct domain *d,
+static int acpi_cpumap_access_common(struct domain *d, bool is_guest_access,
int dir, unsigned int port,
unsigned int bytes, uint32_t *val)
{
@@ -32,14 +34,15 @@ static int acpi_cpumap_access_common(struct domain *d,
memcpy(val, (uint8_t *)d->avail_vcpus + first_byte,
min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
}
- else
+ else if ( !is_guest_access )
/* Guests do not write CPU map */
- return X86EMUL_UNHANDLEABLE;
+ memcpy((uint8_t *)d->avail_vcpus + first_byte, val,
+ min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
return X86EMUL_OKAY;
}
-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)
{
@@ -96,14 +99,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;
/* fallthrough */
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;
@@ -129,13 +138,50 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
const xen_acpi_access_t *access,
XEN_GUEST_HANDLE_PARAM(void) arg)
{
- return -ENOSYS;
+ unsigned int bytes, i;
+ uint32_t val;
+ uint8_t *ptr = (uint8_t *)&val;
+ int rc;
+ int (*do_acpi_access)(struct domain *d, bool is_guest_access,
+ int dir, unsigned int port,
+ unsigned int bytes, uint32_t *val);
+
+ if ( has_acpi_dm_ff(d) )
+ return -EINVAL;
+
+ if ( access->space_id != XEN_ACPI_SYSTEM_IO )
+ return -EINVAL;
+
+ if ( (access->address >= XEN_ACPI_CPU_MAP) &&
+ (access->address < XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN) )
+ do_acpi_access = acpi_cpumap_access_common;
+ else
+ do_acpi_access = acpi_access_common;
+
+ for ( i = 0; i < access->width; i += sizeof(val) )
+ {
+ bytes = (access->width - i > sizeof(val)) ? sizeof(val) : access->width - i;
+
+ if ( (rw == XEN_DOMCTL_ACPI_WRITE) &&
+ copy_from_guest_offset(ptr, arg, i, bytes) )
+ return -EFAULT;
+
+ rc = do_acpi_access(d, false, rw, access->address, 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);
@@ -144,7 +190,7 @@ static int acpi_guest_access(int dir, unsigned int port,
static int acpi_cpumap_guest_access(int dir, unsigned int port,
unsigned int bytes, uint32_t *val)
{
- return acpi_cpumap_access_common(current->domain,
+ return acpi_cpumap_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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl
2016-12-16 23:18 ` [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl Boris Ostrovsky
@ 2016-12-20 13:24 ` Jan Beulich
2016-12-20 14:45 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 13:24 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> @@ -32,14 +34,15 @@ static int acpi_cpumap_access_common(struct domain *d,
> memcpy(val, (uint8_t *)d->avail_vcpus + first_byte,
> min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
> }
> - else
> + else if ( !is_guest_access )
> /* Guests do not write CPU map */
> - return X86EMUL_UNHANDLEABLE;
> + memcpy((uint8_t *)d->avail_vcpus + first_byte, val,
> + min(bytes, ((d->max_vcpus + 7) / 8) - first_byte));
>
> return X86EMUL_OKAY;
> }
So you're changing to return OKAY even in the guest-write case -
I don't think that's what you want.
> -static int acpi_access_common(struct domain *d,
> +static int acpi_access_common(struct domain *d, bool is_guest_access,
Why? I thought the domctl is needed only for updating the CPU
map? Or maybe it would help if the patch had a non-empty
description.
> @@ -129,13 +138,50 @@ int hvm_acpi_domctl_access(struct domain *d, uint8_t rw,
> const xen_acpi_access_t *access,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> {
> - return -ENOSYS;
> + unsigned int bytes, i;
> + uint32_t val;
> + uint8_t *ptr = (uint8_t *)&val;
> + int rc;
> + int (*do_acpi_access)(struct domain *d, bool is_guest_access,
> + int dir, unsigned int port,
> + unsigned int bytes, uint32_t *val);
> +
> + if ( has_acpi_dm_ff(d) )
> + return -EINVAL;
Perhaps better EOPNOTSUPP or ENODEV?
> + if ( access->space_id != XEN_ACPI_SYSTEM_IO )
> + return -EINVAL;
> +
> + if ( (access->address >= XEN_ACPI_CPU_MAP) &&
> + (access->address < XEN_ACPI_CPU_MAP + XEN_ACPI_CPU_MAP_LEN) )
> + do_acpi_access = acpi_cpumap_access_common;
> + else
> + do_acpi_access = acpi_access_common;
> +
> + for ( i = 0; i < access->width; i += sizeof(val) )
> + {
> + bytes = (access->width - i > sizeof(val)) ? sizeof(val) : access->width - i;
> +
> + if ( (rw == XEN_DOMCTL_ACPI_WRITE) &&
> + copy_from_guest_offset(ptr, arg, i, bytes) )
> + return -EFAULT;
> +
> + rc = do_acpi_access(d, false, rw, access->address, bytes, &val);
> + if ( rc )
> + return rc;
> +
> + if ( (rw == XEN_DOMCTL_ACPI_READ) &&
> + copy_to_guest_offset(arg, i, ptr, bytes) )
> + return -EFAULT;
> + }
I could imagine Coverity considering val potentially uninitialized
with this logic, i.e. I think we'd be better off if it had an
initializer.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl
2016-12-20 13:24 ` Jan Beulich
@ 2016-12-20 14:45 ` Boris Ostrovsky
2016-12-20 14:52 ` Jan Beulich
0 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 14:45 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
On 12/20/2016 08:24 AM, Jan Beulich wrote:
>
>> -static int acpi_access_common(struct domain *d,
>> +static int acpi_access_common(struct domain *d, bool is_guest_access,
> Why? I thought the domctl is needed only for updating the CPU
> map? Or maybe it would help if the patch had a non-empty
> description.
domctl updates both the map and the status. I.e. in the toolstack it
looks like
/*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_ACPI_GPE0_CPUHP_BIT;
rc = xc_acpi_iowrite(CTX->xch, domid, ACPI_GPE0_BLK_ADDRESS_V1,
sizeof(val), &val);
}
I'll make a note in the commit message of the fact that both are accessed.
OTOH, maybe we should have an update to the map trigger the SCI and not
require the toolstack to do so?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl
2016-12-20 14:45 ` Boris Ostrovsky
@ 2016-12-20 14:52 ` Jan Beulich
0 siblings, 0 replies; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 14:52 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 20.12.16 at 15:45, <boris.ostrovsky@oracle.com> wrote:
> On 12/20/2016 08:24 AM, Jan Beulich wrote:
>>
>>> -static int acpi_access_common(struct domain *d,
>>> +static int acpi_access_common(struct domain *d, bool is_guest_access,
>> Why? I thought the domctl is needed only for updating the CPU
>> map? Or maybe it would help if the patch had a non-empty
>> description.
>
> domctl updates both the map and the status. I.e. in the toolstack it
> looks like
>
> /*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_ACPI_GPE0_CPUHP_BIT;
> rc = xc_acpi_iowrite(CTX->xch, domid, ACPI_GPE0_BLK_ADDRESS_V1,
> sizeof(val), &val);
> }
>
>
> I'll make a note in the commit message of the fact that both are accessed.
>
> OTOH, maybe we should have an update to the map trigger the SCI and not
> require the toolstack to do so?
That would make sense, I think.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 07/13] events/x86: Define SCI virtual interrupt
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (5 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 06/13] x86/domctl: Handle ACPI access from domctl Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-16 23:18 ` [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event Boris Ostrovsky
` (5 subsequent siblings)
12 siblings, 0 replies; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
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/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 2565acd..b1290a3 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_ACPI_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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (6 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 07/13] events/x86: Define SCI virtual interrupt Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-20 13:37 ` Jan Beulich
2016-12-16 23:18 ` [PATCH v5 09/13] libxl: Update xenstore on VCPU hotplug for all guest types Boris Ostrovsky
` (4 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
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 v5:
* Fixed SCI-triggering logic
xen/arch/x86/hvm/acpi.c | 9 +++++++++
xen/common/event_channel.c | 7 +++++--
xen/include/xen/domain.h | 1 +
xen/include/xen/event.h | 8 ++++++++
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/acpi.c b/xen/arch/x86/hvm/acpi.c
index 044699d..c3c5ed7 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>
@@ -94,6 +95,7 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
else
{
uint32_t v = *val;
+ uint16_t sts_orig = *sts;
/* Status register is write-1-to-clear by guests */
switch ( port & 3 )
@@ -128,6 +130,13 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
*en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
break;
}
+
+ /*
+ * 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) & *en) )
+ send_guest_global_virq(d, VIRQ_SCI);
}
return X86EMUL_OKAY;
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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event
2016-12-16 23:18 ` [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event Boris Ostrovsky
@ 2016-12-20 13:37 ` Jan Beulich
2016-12-20 14:54 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 13:37 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> @@ -128,6 +130,13 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
> *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
> break;
> }
> +
> + /*
> + * 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) & *en) )
> + send_guest_global_virq(d, VIRQ_SCI);
From an abstract pov: How a bit gets set in either register shouldn't
matter. Hence the !is_guest_access part here is at least questionable.
However, iirc the guest can't itself set any status bits (since they're
write-1-clear). Otoh, when an enable bit transitions from 0 to 1 (which
the guest can effect) and the respective status bit is set, an SCI
should occur according to my reading of the spec.
Also - indentation.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event
2016-12-20 13:37 ` Jan Beulich
@ 2016-12-20 14:54 ` Boris Ostrovsky
0 siblings, 0 replies; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 14:54 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
On 12/20/2016 08:37 AM, Jan Beulich wrote:
>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>> @@ -128,6 +130,13 @@ static int acpi_access_common(struct domain *d, bool is_guest_access,
>> *en = (((v & 0xff) << 8) | (*en & 0xff)) & *mask_en;
>> break;
>> }
>> +
>> + /*
>> + * 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) & *en) )
>> + send_guest_global_virq(d, VIRQ_SCI);
> From an abstract pov: How a bit gets set in either register shouldn't
> matter. Hence the !is_guest_access part here is at least questionable.
> However, iirc the guest can't itself set any status bits (since they're
> write-1-clear). Otoh, when an enable bit transitions from 0 to 1 (which
> the guest can effect) and the respective status bit is set, an SCI
> should occur according to my reading of the spec.
I haven't considered the case of enabling with a pending status. Yes,
that should trigger an SCI too, irrespective of is_guest_access.
And I guess since guests can't set status bits I can drop the
is_guest_access test in the check above too.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 09/13] libxl: Update xenstore on VCPU hotplug for all guest types
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (7 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 08/13] pvh: Send an SCI on VCPU hotplug event Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2017-01-04 10:34 ` Wei Liu
2016-12-16 23:18 ` [PATCH v5 10/13] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug Boris Ostrovsky
` (3 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
Currently HVM guests that use upstream qemu do not update xenstore's
availability entry for VCPUs. While it is not strictly necessary for
hotplug to work, xenstore end up reflecting actual status of VCPUs.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
New in v5
tools/libxl/libxl.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 6fd4fe1..bbbb3de 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5148,7 +5148,6 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
switch (libxl__device_model_version_running(gc, domid)) {
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
case LIBXL_DEVICE_MODEL_VERSION_NONE:
- rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
break;
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap, &info);
@@ -5158,11 +5157,14 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
}
break;
case LIBXL_DOMAIN_TYPE_PV:
- rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
break;
default:
rc = ERROR_INVAL;
}
+
+ if (!rc)
+ rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
+
out:
libxl_dominfo_dispose(&info);
GC_FREE;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 09/13] libxl: Update xenstore on VCPU hotplug for all guest types
2016-12-16 23:18 ` [PATCH v5 09/13] libxl: Update xenstore on VCPU hotplug for all guest types Boris Ostrovsky
@ 2017-01-04 10:34 ` Wei Liu
2017-01-04 13:53 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Wei Liu @ 2017-01-04 10:34 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: wei.liu2, andrew.cooper3, ian.jackson, xen-devel, jbeulich,
roger.pau
On Fri, Dec 16, 2016 at 06:18:35PM -0500, Boris Ostrovsky wrote:
> Currently HVM guests that use upstream qemu do not update xenstore's
> availability entry for VCPUs. While it is not strictly necessary for
> hotplug to work, xenstore end up reflecting actual status of VCPUs.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
This patch is self-contained, so I will just commit it at some point. I
will wait a bit to see if there is objection.
> ---
> New in v5
>
> tools/libxl/libxl.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 6fd4fe1..bbbb3de 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5148,7 +5148,6 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
> switch (libxl__device_model_version_running(gc, domid)) {
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> case LIBXL_DEVICE_MODEL_VERSION_NONE:
> - rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
> break;
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap, &info);
> @@ -5158,11 +5157,14 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
> }
> break;
> case LIBXL_DOMAIN_TYPE_PV:
> - rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
> break;
> default:
> rc = ERROR_INVAL;
> }
> +
> + if (!rc)
> + rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
> +
> out:
> libxl_dominfo_dispose(&info);
> GC_FREE;
> --
> 2.7.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 09/13] libxl: Update xenstore on VCPU hotplug for all guest types
2017-01-04 10:34 ` Wei Liu
@ 2017-01-04 13:53 ` Boris Ostrovsky
0 siblings, 0 replies; 44+ messages in thread
From: Boris Ostrovsky @ 2017-01-04 13:53 UTC (permalink / raw)
To: Wei Liu; +Cc: andrew.cooper3, roger.pau, ian.jackson, jbeulich, xen-devel
On 01/04/2017 05:34 AM, Wei Liu wrote:
> On Fri, Dec 16, 2016 at 06:18:35PM -0500, Boris Ostrovsky wrote:
>> Currently HVM guests that use upstream qemu do not update xenstore's
>> availability entry for VCPUs. While it is not strictly necessary for
>> hotplug to work, xenstore end up reflecting actual status of VCPUs.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
>
> This patch is self-contained, so I will just commit it at some point. I
> will wait a bit to see if there is objection.
>
>> ---
>> New in v5
Yes, this patch (but v6, which you also acked) can go in independent of
the rest of the series, which is on hold for now.
-boris
>>
>> tools/libxl/libxl.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
>> index 6fd4fe1..bbbb3de 100644
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -5148,7 +5148,6 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
>> switch (libxl__device_model_version_running(gc, domid)) {
>> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
>> case LIBXL_DEVICE_MODEL_VERSION_NONE:
>> - rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
>> break;
>> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>> rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap, &info);
>> @@ -5158,11 +5157,14 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
>> }
>> break;
>> case LIBXL_DOMAIN_TYPE_PV:
>> - rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
>> break;
>> default:
>> rc = ERROR_INVAL;
>> }
>> +
>> + if (!rc)
>> + rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, &info);
>> +
>> out:
>> libxl_dominfo_dispose(&info);
>> GC_FREE;
>> --
>> 2.7.4
>>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 10/13] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (8 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 09/13] libxl: Update xenstore on VCPU hotplug for all guest types Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2017-01-04 10:35 ` Wei Liu
2016-12-16 23:18 ` [PATCH v5 11/13] pvh: Set online VCPU map to avail_vcpus Boris Ostrovsky
` (2 subsequent siblings)
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
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 v5:
* xc_acpi_access() changes due to new interface structure
* Set VCPU online map during domain creation (in libxl__build_pre())
tools/libxc/include/xenctrl.h | 20 ++++++++++++++++++++
tools/libxc/xc_domain.c | 37 +++++++++++++++++++++++++++++++++++++
tools/libxl/libxl.c | 4 ++++
tools/libxl/libxl_arch.h | 4 ++++
tools/libxl/libxl_arm.c | 6 ++++++
tools/libxl/libxl_dom.c | 10 ++++++++++
tools/libxl/libxl_x86.c | 21 +++++++++++++++++++++
7 files changed, 102 insertions(+)
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..c038932 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -2520,6 +2520,43 @@ 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);
+ struct xen_acpi_access *access = &domctl.u.acpi_access.access;
+ unsigned max_bytes = (1U << (sizeof(access->width) * 8)) - 1;
+
+ while ( 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;
+ access->space_id = space_id;
+ access->width = bytes < max_bytes ? bytes : max_bytes;
+ access->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 -= access->width;
+ address += access->width;
+ }
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index bbbb3de..d8306ff 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5147,7 +5147,11 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid, libxl_bitmap *cpumap)
case LIBXL_DOMAIN_TYPE_HVM:
switch (libxl__device_model_version_running(gc, domid)) {
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+ break;
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);
break;
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
rc = libxl__set_vcpuonline_qmp(gc, domid, cpumap, &info);
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
index 5e1fc60..9649c21 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..93dc81e 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_dom.c b/tools/libxl/libxl_dom.c
index d519c8d..ca8f7a2 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -309,6 +309,16 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
return ERROR_FAIL;
}
+ if ((info->type == LIBXL_DOMAIN_TYPE_HVM) &&
+ (libxl__device_model_version_running(gc, domid) ==
+ LIBXL_DEVICE_MODEL_VERSION_NONE)) {
+ rc = libxl__arch_set_vcpuonline(gc, domid, &info->avail_vcpus);
+ if (rc) {
+ LOG(ERROR, "Couldn't set available vcpu count (error %d)", rc);
+ return ERROR_FAIL;
+ }
+ }
+
/*
* Check if the domain has any CPU or node affinity already. If not, try
* to build up the latter via automatic NUMA placement. In fact, in case
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 5da7504..c06a2d5 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_ACPI_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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 10/13] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug
2016-12-16 23:18 ` [PATCH v5 10/13] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug Boris Ostrovsky
@ 2017-01-04 10:35 ` Wei Liu
0 siblings, 0 replies; 44+ messages in thread
From: Wei Liu @ 2017-01-04 10:35 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: wei.liu2, andrew.cooper3, ian.jackson, xen-devel, jbeulich,
roger.pau
On Fri, Dec 16, 2016 at 06:18:36PM -0500, Boris Ostrovsky wrote:
> 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>
Since there is objection on how CPU hotplug should be handled, I will
just skip this patch for now. Let me know if you think otherwise.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 11/13] pvh: Set online VCPU map to avail_vcpus
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (9 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 10/13] tools: Call XEN_DOMCTL_acpi_access on PVH VCPU hotplug Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-16 23:18 ` [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests Boris Ostrovsky
2016-12-16 23:18 ` [PATCH v5 13/13] docs: Describe PVHv2's VCPU hotplug procedure Boris Ostrovsky
12 siblings, 0 replies; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
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
^ permalink raw reply related [flat|nested] 44+ messages in thread* [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (10 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 11/13] pvh: Set online VCPU map to avail_vcpus Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
2016-12-20 13:57 ` Jan Beulich
2016-12-16 23:18 ` [PATCH v5 13/13] docs: Describe PVHv2's VCPU hotplug procedure Boris Ostrovsky
12 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: wei.liu2, andrew.cooper3, ian.jackson, jbeulich, Boris Ostrovsky,
roger.pau
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
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 b70c299..37acf11 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(PMTIMER, 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(PMTIMER, 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
^ permalink raw reply related [flat|nested] 44+ messages in thread* Re: [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests
2016-12-16 23:18 ` [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests Boris Ostrovsky
@ 2016-12-20 13:57 ` Jan Beulich
2016-12-20 15:09 ` Boris Ostrovsky
0 siblings, 1 reply; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 13:57 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
> --- 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(PMTIMER, 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(PMTIMER, h, acpi);
> return -ENODEV;
> + }
>
> spin_lock(&s->lock);
Seeing this I first of all wonder - would there be any harm in simply
having PVH take (almost) the same route as HVM here? In particular
there's a pmt_update_sci() call, an equivalent of which would seem
to be needed for PVH too.
Which in turn gets me to wonder whether some of the code which
is already there couldn't be re-used (handle_evt_io() for example).
And then, seeing the locking here - don't you need some locking
in the earlier patches too, both to serialize accesses from multiple
guest vCPU-s and to arbitrate between Dom0 and the guest?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests
2016-12-20 13:57 ` Jan Beulich
@ 2016-12-20 15:09 ` Boris Ostrovsky
2016-12-20 15:40 ` Jan Beulich
0 siblings, 1 reply; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-20 15:09 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
On 12/20/2016 08:57 AM, Jan Beulich wrote:
>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>> --- 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(PMTIMER, 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(PMTIMER, h, acpi);
>> return -ENODEV;
>> + }
>>
>> spin_lock(&s->lock);
> Seeing this I first of all wonder - would there be any harm in simply
> having PVH take (almost) the same route as HVM here? In particular
> there's a pmt_update_sci() call, an equivalent of which would seem
> to be needed for PVH too.
It probably is harmless but not very useful too since we'd be saving
PMTState which is not in use by PVH.
As far as pmt_update_sci() --- the only case when we have an SCI
generated is CPU map update and the interrupt is made pending to the
guest immediately so if we try to resend it during restore won't we be
sending it twice?
>
> Which in turn gets me to wonder whether some of the code which
> is already there couldn't be re-used (handle_evt_io() for example).
My plan is to merge these routines (to some extent) when we make HVM
guests use the same CPU hotplug code as PVH (i.e. stop using QEMU for that).
> And then, seeing the locking here - don't you need some locking
> in the earlier patches too, both to serialize accesses from multiple
> guest vCPU-s and to arbitrate between Dom0 and the guest?
Yes, that was missing.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread* Re: [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests
2016-12-20 15:09 ` Boris Ostrovsky
@ 2016-12-20 15:40 ` Jan Beulich
0 siblings, 0 replies; 44+ messages in thread
From: Jan Beulich @ 2016-12-20 15:40 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: andrew.cooper3, xen-devel, wei.liu2, ian.jackson, roger.pau
>>> On 20.12.16 at 16:09, <boris.ostrovsky@oracle.com> wrote:
> On 12/20/2016 08:57 AM, Jan Beulich wrote:
>>>>> On 17.12.16 at 00:18, <boris.ostrovsky@oracle.com> wrote:
>>> --- 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(PMTIMER, 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(PMTIMER, h, acpi);
>>> return -ENODEV;
>>> + }
>>>
>>> spin_lock(&s->lock);
>> Seeing this I first of all wonder - would there be any harm in simply
>> having PVH take (almost) the same route as HVM here? In particular
>> there's a pmt_update_sci() call, an equivalent of which would seem
>> to be needed for PVH too.
>
> It probably is harmless but not very useful too since we'd be saving
> PMTState which is not in use by PVH.
>
> As far as pmt_update_sci() --- the only case when we have an SCI
> generated is CPU map update and the interrupt is made pending to the
> guest immediately so if we try to resend it during restore won't we be
> sending it twice?
Well, depends on how far things got prior to the (racing) migration.
I think it's better to send one too many than risking the guest not
getting to see any.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v5 13/13] docs: Describe PVHv2's VCPU hotplug procedure
2016-12-16 23:18 [PATCH v5 00/13] PVH VCPU hotplug support Boris Ostrovsky
` (11 preceding siblings ...)
2016-12-16 23:18 ` [PATCH v5 12/13] pvh/acpi: Save ACPI registers for PVH guests Boris Ostrovsky
@ 2016-12-16 23:18 ` Boris Ostrovsky
12 siblings, 0 replies; 44+ messages in thread
From: Boris Ostrovsky @ 2016-12-16 23:18 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, wei.liu2, George Dunlap, andrew.cooper3,
ian.jackson, Tim Deegan, jbeulich, Boris Ostrovsky, roger.pau
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>
---
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
^ permalink raw reply related [flat|nested] 44+ messages in thread