diff mbox

[v4,4/6] x86/PV: allow PV guests to have an emulated PIT

Message ID 1453395092-88090-5-git-send-email-roger.pau@citrix.com
State New, archived
Headers show

Commit Message

Roger Pau Monné Jan. 21, 2016, 4:51 p.m. UTC
This fixes the fallout from the HVMlite series, that removed the emulated
PIT from PV(H) guests. Also, this patch forces the hardware domain to
always have an emulated PIT, regardless of whether the toolstack specified
one or not.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v3:
 - Allow PV guests to specify whether a PIT is needed or not.
 - Fix pv_pit_handler so it can deal with the fact that the PIT might not be
   present.

Changes since v2:
 - Force the emulated PIT to always be enabled for the hardware domain.
 - Change indentation of the valid set of emulation bitmaps check.
---
 xen/arch/x86/domain.c    | 5 ++++-
 xen/arch/x86/hvm/i8254.c | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

Comments

Jan Beulich Jan. 22, 2016, 10:48 a.m. UTC | #1
>>> On 21.01.16 at 17:51, <roger.pau@citrix.com> wrote:
> This fixes the fallout from the HVMlite series, that removed the emulated
> PIT from PV(H) guests. Also, this patch forces the hardware domain to
> always have an emulated PIT, regardless of whether the toolstack specified
> one or not.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

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

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -542,8 +542,11 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>                     d->domain_id, config->emulation_flags);
>              return -EINVAL;
>          }
> +        if ( is_hardware_domain(d) )
> +            config->emulation_flags |= XEN_X86_EMU_PIT;
>          if ( config->emulation_flags != 0 &&
> -             (!is_hvm_domain(d) || config->emulation_flags != XEN_X86_EMU_ALL) )
> +             (is_hvm_domain(d) ? (config->emulation_flags != XEN_X86_EMU_ALL) :
> +                                 (config->emulation_flags != XEN_X86_EMU_PIT)) )
>          {

... you having chosen the != route instead of the suggested &
one, I guess I'll take the liberty to further simplify this while
committing (as the ?: is now only needed on the right side of the
!= and I'm generally of the opinion that redundancy like this is
hampering readability).

Jan
Ian Campbell Jan. 22, 2016, 11:03 a.m. UTC | #2
On Fri, 2016-01-22 at 03:48 -0700, Jan Beulich wrote:
> > > > On 21.01.16 at 17:51, <roger.pau@citrix.com> wrote:
> > This fixes the fallout from the HVMlite series, that removed the
> > emulated
> > PIT from PV(H) guests. Also, this patch forces the hardware domain to
> > always have an emulated PIT, regardless of whether the toolstack
> > specified
> > one or not.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> albeit ...
> 
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -542,8 +542,11 @@ int arch_domain_create(struct domain *d, unsigned
> > int 
> > domcr_flags,
> >                     d->domain_id, config->emulation_flags);
> >              return -EINVAL;
> >          }
> > +        if ( is_hardware_domain(d) )
> > +            config->emulation_flags |= XEN_X86_EMU_PIT;
> >          if ( config->emulation_flags != 0 &&
> > -             (!is_hvm_domain(d) || config->emulation_flags !=
> > XEN_X86_EMU_ALL) )
> > +             (is_hvm_domain(d) ? (config->emulation_flags !=
> > XEN_X86_EMU_ALL) :
> > +                                 (config->emulation_flags !=
> > XEN_X86_EMU_PIT)) )
> >          {
> 
> ... you having chosen the != route instead of the suggested &
> one, I guess I'll take the liberty to further simplify this while
> committing (as the ?: is now only needed on the right side of the
> != and I'm generally of the opinion that redundancy like this is
> hampering readability).

Readability would be even greater IMHO with the use of a
"required_emulation_flags" variable suitably initialised and then checked,
compared with the use of bitops etc.

> 
> Jan
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
diff mbox

Patch

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index e70c125..b005fe0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -542,8 +542,11 @@  int arch_domain_create(struct domain *d, unsigned int domcr_flags,
                    d->domain_id, config->emulation_flags);
             return -EINVAL;
         }
+        if ( is_hardware_domain(d) )
+            config->emulation_flags |= XEN_X86_EMU_PIT;
         if ( config->emulation_flags != 0 &&
-             (!is_hvm_domain(d) || config->emulation_flags != XEN_X86_EMU_ALL) )
+             (is_hvm_domain(d) ? (config->emulation_flags != XEN_X86_EMU_ALL) :
+                                 (config->emulation_flags != XEN_X86_EMU_PIT)) )
         {
             printk(XENLOG_G_ERR "d%d: Xen does not allow %s domain creation "
                    "with the current selection of emulators: %#x\n",
diff --git a/xen/arch/x86/hvm/i8254.c b/xen/arch/x86/hvm/i8254.c
index b517cd6..577b43c 100644
--- a/xen/arch/x86/hvm/i8254.c
+++ b/xen/arch/x86/hvm/i8254.c
@@ -568,6 +568,9 @@  int pv_pit_handler(int port, int data, int write)
         .data = data
     };
 
+    if ( !has_vpit(current->domain) )
+        return ~0;
+
     if ( is_hardware_domain(current->domain) && hwdom_pit_access(&ioreq) )
     {
         /* nothing to do */;