From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] x86/HVM: tie RTC emulation mode to enabling of Viridian emulation Date: Tue, 2 Jul 2013 11:10:26 +0100 Message-ID: <51D2A712.4030806@citrix.com> References: <51D2973102000078000E21A5@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5660562456506609605==" Return-path: In-Reply-To: <51D2973102000078000E21A5@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: George Dunlap , Keir Fraser , paul.durrant@citrix.com, Tim Deegan , xen-devel List-Id: xen-devel@lists.xenproject.org --===============5660562456506609605== Content-Type: multipart/alternative; boundary="------------070302060408060707060009" --------------070302060408060707060009 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit On 02/07/13 08:02, Jan Beulich wrote: > As the mode not conforming to the hardware specification (by allowing > the guest to skip the REG C reads in its interrupt handler) is a > Viridian invention, it seems logical to tie this mode to that extension > being enabled. If the extension is disabled, proper hardware emulation > will be done instead. > > The main thing necessary here is the synchronization of the RTC > emulation code and the setting of the respective flag in hvmloader's > creation of the ACPI WAET table. > > Signed-off-by: Jan Beulich This looks to be safe migrating forwards Reviewed-by: Andrew Cooper > > --- a/tools/firmware/hvmloader/acpi/acpi2_0.h > +++ b/tools/firmware/hvmloader/acpi/acpi2_0.h > @@ -304,6 +304,9 @@ struct acpi_20_waet { > uint32_t flags; > }; > > +#define ACPI_WAET_RTC_NO_ACK (1<<0) /* RTC requires no int acknowledge */ > +#define ACPI_WAET_TIMER_ONE_READ (1<<1) /* PM timer requires only one read */ > + > /* > * Multiple APIC Flags. > */ > --- a/tools/firmware/hvmloader/acpi/build.c > +++ b/tools/firmware/hvmloader/acpi/build.c > @@ -22,8 +22,10 @@ > #include "ssdt_tpm.h" > #include "ssdt_pm.h" > #include "../config.h" > +#include "../hypercall.h" > #include "../util.h" > #include > +#include > > #define ACPI_MAX_SECONDARY_TABLES 16 > > @@ -189,6 +191,7 @@ static struct acpi_20_hpet *construct_hp > static struct acpi_20_waet *construct_waet(void) > { > struct acpi_20_waet *waet; > + xen_hvm_param_t param; > > waet = mem_alloc(sizeof(*waet), 16); > if (!waet) return NULL; > @@ -196,6 +199,19 @@ static struct acpi_20_waet *construct_wa > memcpy(waet, &Waet, sizeof(*waet)); > > waet->header.length = sizeof(*waet); > + > + /* > + * Check whether Viridian emulation is enabled: The state of the RTC > + * flag getting passed to the guest must be in sync with the mode > + * selection in the hypervisor RTC emulation code. > + */ > + param.domid = DOMID_SELF; > + param.index = HVM_PARAM_VIRIDIAN; > + if ( hypercall_hvm_op(HVMOP_get_param, ¶m) ) > + BUG(); > + if ( param.value ) > + waet->flags |= ACPI_WAET_RTC_NO_ACK; > + > set_checksum(waet, offsetof(struct acpi_header, checksum), sizeof(*waet)); > > return waet; > --- a/tools/firmware/hvmloader/acpi/static_tables.c > +++ b/tools/firmware/hvmloader/acpi/static_tables.c > @@ -136,16 +136,6 @@ struct acpi_20_rsdp Rsdp = { > .length = sizeof(struct acpi_20_rsdp) > }; > > -#define ACPI_WAET_RTC_NO_ACK (1<<0) /* RTC requires no int acknowledge */ > -#define ACPI_WAET_TIMER_ONE_READ (1<<1) /* PM timer requires only one read */ > - > -/* > - * The state of the RTC flag getting passed to the guest must be in > - * sync with the mode selection in the hypervisor RTC emulation code. > - */ > -#define ACPI_WAET_FLAGS (ACPI_WAET_RTC_NO_ACK | \ > - ACPI_WAET_TIMER_ONE_READ) > - > struct acpi_20_waet Waet = { > .header = { > .signature = ACPI_2_0_WAET_SIGNATURE, > @@ -157,7 +147,7 @@ struct acpi_20_waet Waet = { > .creator_id = ACPI_CREATOR_ID, > .creator_revision = ACPI_CREATOR_REVISION > }, > - .flags = ACPI_WAET_FLAGS > + .flags = ACPI_WAET_TIMER_ONE_READ > }; > > /* > --- a/xen/arch/x86/hvm/rtc.c > +++ b/xen/arch/x86/hvm/rtc.c > @@ -51,7 +51,9 @@ enum rtc_mode { > }; > > /* This must be in sync with how hvmloader sets the ACPI WAET flags. */ > -#define mode_is(d, m) ((void)(d), rtc_mode_##m == rtc_mode_no_ack) > +#define mode_is(d, m) (is_viridian_domain(d) ? \ > + rtc_mode_##m == rtc_mode_no_ack : \ > + rtc_mode_##m == rtc_mode_strict) > #define rtc_mode_is(s, m) mode_is(vrtc_domain(s), m) > > static void rtc_copy_date(RTCState *s); > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------070302060408060707060009 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 02/07/13 08:02, Jan Beulich wrote:
As the mode not conforming to the hardware specification (by allowing
the guest to skip the REG C reads in its interrupt handler) is a
Viridian invention, it seems logical to tie this mode to that extension
being enabled. If the extension is disabled, proper hardware emulation
will be done instead.

The main thing necessary here is the synchronization of the RTC
emulation code and the setting of the respective flag in hvmloader's
creation of the ACPI WAET table.

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

This looks to be safe migrating forwards

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


--- a/tools/firmware/hvmloader/acpi/acpi2_0.h
+++ b/tools/firmware/hvmloader/acpi/acpi2_0.h
@@ -304,6 +304,9 @@ struct acpi_20_waet {
     uint32_t           flags;
 };
 
+#define ACPI_WAET_RTC_NO_ACK        (1<<0) /* RTC requires no int acknowledge */
+#define ACPI_WAET_TIMER_ONE_READ    (1<<1) /* PM timer requires only one read */
+
 /*
  * Multiple APIC Flags.
  */
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -22,8 +22,10 @@
 #include "ssdt_tpm.h"
 #include "ssdt_pm.h"
 #include "../config.h"
+#include "../hypercall.h"
 #include "../util.h"
 #include <xen/hvm/hvm_xs_strings.h>
+#include <xen/hvm/params.h>
 
 #define ACPI_MAX_SECONDARY_TABLES 16
 
@@ -189,6 +191,7 @@ static struct acpi_20_hpet *construct_hp
 static struct acpi_20_waet *construct_waet(void)
 {
     struct acpi_20_waet *waet;
+    xen_hvm_param_t param;
 
     waet = mem_alloc(sizeof(*waet), 16);
     if (!waet) return NULL;
@@ -196,6 +199,19 @@ static struct acpi_20_waet *construct_wa
     memcpy(waet, &Waet, sizeof(*waet));
 
     waet->header.length = sizeof(*waet);
+
+    /*
+     * Check whether Viridian emulation is enabled: The state of the RTC
+     * flag getting passed to the guest must be in sync with the mode
+     * selection in the hypervisor RTC emulation code.
+     */
+    param.domid = DOMID_SELF;
+    param.index = HVM_PARAM_VIRIDIAN;
+    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
+        BUG();
+    if ( param.value )
+        waet->flags |= ACPI_WAET_RTC_NO_ACK;
+
     set_checksum(waet, offsetof(struct acpi_header, checksum), sizeof(*waet));
 
     return waet;
--- a/tools/firmware/hvmloader/acpi/static_tables.c
+++ b/tools/firmware/hvmloader/acpi/static_tables.c
@@ -136,16 +136,6 @@ struct acpi_20_rsdp Rsdp = {
     .length    = sizeof(struct acpi_20_rsdp)
 };
 
-#define ACPI_WAET_RTC_NO_ACK        (1<<0) /* RTC requires no int acknowledge */
-#define ACPI_WAET_TIMER_ONE_READ    (1<<1) /* PM timer requires only one read */
-
-/*
- * The state of the RTC flag getting passed to the guest must be in
- * sync with the mode selection in the hypervisor RTC emulation code.
- */
-#define ACPI_WAET_FLAGS (ACPI_WAET_RTC_NO_ACK | \
-                         ACPI_WAET_TIMER_ONE_READ)
-
 struct acpi_20_waet Waet = {
     .header = {
         .signature    = ACPI_2_0_WAET_SIGNATURE,
@@ -157,7 +147,7 @@ struct acpi_20_waet Waet = {
         .creator_id   = ACPI_CREATOR_ID,
         .creator_revision = ACPI_CREATOR_REVISION
     },
-    .flags = ACPI_WAET_FLAGS
+    .flags = ACPI_WAET_TIMER_ONE_READ
 };
 
 /*
--- a/xen/arch/x86/hvm/rtc.c
+++ b/xen/arch/x86/hvm/rtc.c
@@ -51,7 +51,9 @@ enum rtc_mode {
 };
 
 /* This must be in sync with how hvmloader sets the ACPI WAET flags. */
-#define mode_is(d, m) ((void)(d), rtc_mode_##m == rtc_mode_no_ack)
+#define mode_is(d, m) (is_viridian_domain(d) ? \
+                       rtc_mode_##m == rtc_mode_no_ack : \
+                       rtc_mode_##m == rtc_mode_strict)
 #define rtc_mode_is(s, m) mode_is(vrtc_domain(s), m)
 
 static void rtc_copy_date(RTCState *s);






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

--------------070302060408060707060009-- --===============5660562456506609605== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5660562456506609605==--