hvm: Improve in-Xen PIO emulation to better handle string PIO

instructions.
Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

xen/arch/x86/hvm/i8254.c

@@ -48,8 +48,10 @@
 #define RW_STATE_WORD0 3
 #define RW_STATE_WORD1 4
 
-static int handle_pit_io(ioreq_t *p);
-static int handle_speaker_io(ioreq_t *p);
+static int handle_pit_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val);
+static int handle_speaker_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val);
 
 /* Compute with 96 bit intermediate result: (a*b)/c */
 static uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
@@ -525,24 +527,25 @@ void pit_deinit(struct domain *d)
 }
 
 /* the intercept action for PIT DM retval:0--not handled; 1--handled */  
-static int handle_pit_io(ioreq_t *p)
+static int handle_pit_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct PITState *vpit = vcpu_vpit(current);
 
-    if ( (p->size != 1) || p->data_is_ptr || (p->type != IOREQ_TYPE_PIO) )
+    if ( bytes != 1 )
     {
         gdprintk(XENLOG_WARNING, "PIT bad access\n");
         return 1;
     }
 
-    if ( p->dir == IOREQ_WRITE )
+    if ( dir == IOREQ_WRITE )
     {
-        pit_ioport_write(vpit, p->addr, p->data);
+        pit_ioport_write(vpit, port, *val);
     }
     else
     {
-        if ( (p->addr & 3) != 3 )
-            p->data = pit_ioport_read(vpit, p->addr);
+        if ( (port & 3) != 3 )
+            *val = pit_ioport_read(vpit, port);
         else
             gdprintk(XENLOG_WARNING, "PIT: read A1:A0=3!\n");
     }
@@ -566,11 +569,12 @@ static uint32_t speaker_ioport_read(
             (pit_get_out(pit, 2) << 5) | (refresh_clock << 4));
 }
 
-static int handle_speaker_io(ioreq_t *p)
+static int handle_speaker_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct PITState *vpit = vcpu_vpit(current);
 
-    if ( (p->size != 1) || p->data_is_ptr || (p->type != IOREQ_TYPE_PIO) )
+    if ( bytes != 1 )
     {
         gdprintk(XENLOG_WARNING, "PIT_SPEAKER bad access\n");
         return 1;
@@ -578,10 +582,10 @@ static int handle_speaker_io(ioreq_t *p)
 
     spin_lock(&vpit->lock);
 
-    if ( p->dir == IOREQ_WRITE )
-        speaker_ioport_write(vpit, p->addr, p->data);
+    if ( dir == IOREQ_WRITE )
+        speaker_ioport_write(vpit, port, *val);
     else
-        p->data = speaker_ioport_read(vpit, p->addr);
+        *val = speaker_ioport_read(vpit, port);
 
     spin_unlock(&vpit->lock);
 
@@ -597,13 +601,14 @@ int pv_pit_handler(int port, int data, int write)
         .dir  = write ? IOREQ_WRITE : IOREQ_READ,
         .data = data
     };
+    uint32_t val = data;
 
     if ( (current->domain->domain_id == 0) && dom0_pit_access(&ioreq) )
         /* nothing to do */;
     else if ( port == 0x61 )
-        handle_speaker_io(&ioreq);
+        handle_speaker_io(ioreq.dir, port, 1, &val);
     else
-        handle_pit_io(&ioreq);
+        handle_pit_io(ioreq.dir, port, 1, &val);
 
     return !write ? ioreq.data : 0;
 }

xen/arch/x86/hvm/intercept.c

@@ -247,6 +247,50 @@ int hvm_mmio_intercept(ioreq_t *p)
     return 0;
 }
 
+static int process_portio_intercept(portio_action_t action, ioreq_t *p)
+{
+    int rc = 1, i, sign = p->df ? -1 : 1;
+    uint32_t data;
+
+    if ( p->dir == IOREQ_READ )
+    {
+        if ( !p->data_is_ptr )
+        {
+            rc = action(IOREQ_READ, p->addr, p->size, &data);
+            p->data = data;
+        }
+        else
+        {
+            for ( i = 0; i < p->count; i++ )
+            {
+                rc = action(IOREQ_READ, p->addr, p->size, &data);
+                (void)hvm_copy_to_guest_phys(p->data + sign*i*p->size,
+                                             &data, p->size);
+            }
+        }
+    }
+    else /* p->dir == IOREQ_WRITE */
+    {
+        if ( !p->data_is_ptr )
+        {
+            data = p->data;
+            rc = action(IOREQ_WRITE, p->addr, p->size, &data);
+        }
+        else
+        {
+            for ( i = 0; i < p->count; i++ )
+            {
+                data = 0;
+                (void)hvm_copy_from_guest_phys(&data, p->data + sign*i*p->size,
+                                               p->size);
+                rc = action(IOREQ_WRITE, p->addr, p->size, &data);
+            }
+        }
+    }
+
+    return rc;
+}
+
 /*
  * Check if the request is handled inside xen
  * return value: 0 --not handled; 1 --handled
@@ -255,28 +299,35 @@ int hvm_io_intercept(ioreq_t *p, int type)
 {
     struct vcpu *v = current;
     struct hvm_io_handler *handler =
-                           &(v->domain->arch.hvm_domain.io_handler);
+        &v->domain->arch.hvm_domain.io_handler;
     int i;
     unsigned long addr, size;
 
     if ( (type == HVM_PORTIO) && (dpci_ioport_intercept(p)) )
         return 1;
 
-    for (i = 0; i < handler->num_slot; i++) {
-        if( type != handler->hdl_list[i].type)
+    for ( i = 0; i < handler->num_slot; i++ )
+    {
+        if ( type != handler->hdl_list[i].type )
             continue;
         addr = handler->hdl_list[i].addr;
         size = handler->hdl_list[i].size;
-        if (p->addr >= addr &&
-            p->addr + p->size <=  addr + size)
-            return handler->hdl_list[i].action(p);
+        if ( (p->addr >= addr) &&
+             ((p->addr + p->size) <= (addr + size)) )
+        {
+            if ( type == HVM_PORTIO )
+                return process_portio_intercept(
+                    handler->hdl_list[i].action.portio, p);
+            return handler->hdl_list[i].action.mmio(p);
+        }
     }
+
     return 0;
 }
 
 int register_io_handler(
     struct domain *d, unsigned long addr, unsigned long size,
-    intercept_action_t action, int type)
+    void *action, int type)
 {
     struct hvm_io_handler *handler = &d->arch.hvm_domain.io_handler;
     int num = handler->num_slot;
@@ -285,8 +336,10 @@ int register_io_handler(
 
     handler->hdl_list[num].addr = addr;
     handler->hdl_list[num].size = size;
-    handler->hdl_list[num].action = action;
-    handler->hdl_list[num].type = type;
+    if ( (handler->hdl_list[num].type = type) == HVM_PORTIO )
+        handler->hdl_list[num].action.portio = action;
+    else
+        handler->hdl_list[num].action.mmio = action;
     handler->num_slot++;
 
     return 1;

xen/arch/x86/hvm/pmtimer.c

@@ -114,7 +114,8 @@ static void pmt_timer_callback(void *opaque)
 }
 
 /* Handle port I/O to the PM1a_STS and PM1a_EN registers */
-static int handle_evt_io(ioreq_t *p)
+static int handle_evt_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct vcpu *v = current;
     PMTState *s = &v->domain->arch.hvm_domain.pl_time.vpmt;
@@ -123,10 +124,10 @@ static int handle_evt_io(ioreq_t *p)
 
     spin_lock(&s->lock);
 
-    if ( p->dir == IOREQ_WRITE )
+    if ( dir == IOREQ_WRITE )
     {
         /* Handle this I/O one byte at a time */
-        for ( i = p->size, addr = p->addr, data = p->data;
+        for ( i = bytes, addr = port, data = *val;
               i > 0;
               i--, addr++, data >>= 8 )
         {
@@ -150,9 +151,8 @@ static int handle_evt_io(ioreq_t *p)
                 
             default:
                 gdprintk(XENLOG_WARNING, 
-                         "Bad ACPI PM register write: %"PRIu64
-                         " bytes (%#"PRIx64") at %"PRIx64"\n", 
-                         p->size, p->data, p->addr);
+                         "Bad ACPI PM register write: %x bytes (%x) at %x\n", 
+                         bytes, *val, port);
             }
         }
         /* Fix up the SCI state to match the new register state */
@@ -161,10 +161,10 @@ static int handle_evt_io(ioreq_t *p)
     else /* p->dir == IOREQ_READ */
     {
         data = s->pm.pm1a_sts | (((uint32_t) s->pm.pm1a_en) << 16);
-        data >>= 8 * (p->addr - PM1a_STS_ADDR);
-        if ( p->size == 1 ) data &= 0xff;
-        else if ( p->size == 2 ) data &= 0xffff;
-        p->data = data;
+        data >>= 8 * (port - PM1a_STS_ADDR);
+        if ( bytes == 1 ) data &= 0xff;
+        else if ( bytes == 2 ) data &= 0xffff;
+        *val = data;
     }
 
     spin_unlock(&s->lock);
@@ -174,22 +174,23 @@ static int handle_evt_io(ioreq_t *p)
 
 
 /* Handle port I/O to the TMR_VAL register */
-static int handle_pmt_io(ioreq_t *p)
+static int handle_pmt_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct vcpu *v = current;
     PMTState *s = &v->domain->arch.hvm_domain.pl_time.vpmt;
 
-    if ( (p->size != 4) || p->data_is_ptr || (p->type != IOREQ_TYPE_PIO) )
+    if ( bytes != 4 )
     {
         gdprintk(XENLOG_WARNING, "HVM_PMT bad access\n");
         return 1;
     }
     
-    if ( p->dir == IOREQ_READ )
+    if ( dir == IOREQ_READ )
     {
         spin_lock(&s->lock);
         pmt_update_time(s);
-        p->data = s->pm.tmr_val;
+        *val = s->pm.tmr_val;
         spin_unlock(&s->lock);
         return 1;
     }

xen/arch/x86/hvm/rtc.c

@@ -395,24 +395,25 @@ static uint32_t rtc_ioport_read(RTCState *s, uint32_t addr)
     return ret;
 }
 
-static int handle_rtc_io(ioreq_t *p)
+static int handle_rtc_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct RTCState *vrtc = vcpu_vrtc(current);
 
-    if ( (p->size != 1) || p->data_is_ptr || (p->type != IOREQ_TYPE_PIO) )
+    if ( bytes != 1 )
     {
         gdprintk(XENLOG_WARNING, "HVM_RTC bas access\n");
         return 1;
     }
     
-    if ( p->dir == IOREQ_WRITE )
+    if ( dir == IOREQ_WRITE )
     {
-        if ( rtc_ioport_write(vrtc, p->addr, p->data & 0xFF) )
+        if ( rtc_ioport_write(vrtc, port, (uint8_t)*val) )
             return 1;
     }
     else if ( vrtc->hw.cmos_index < RTC_CMOS_SIZE )
     {
-        p->data = rtc_ioport_read(vrtc, p->addr);
+        *val = rtc_ioport_read(vrtc, port);
         return 1;
     }
 

xen/arch/x86/hvm/stdvga.c

@@ -148,42 +148,37 @@ static int stdvga_outb(uint64_t addr, uint8_t val)
     return rc;
 }
 
-static int stdvga_out(ioreq_t *p)
+static void stdvga_out(uint32_t port, uint32_t bytes, uint32_t val)
 {
-    int rc = 1;
-
-    switch ( p->size )
+    switch ( bytes )
     {
     case 1:
-        rc &= stdvga_outb(p->addr, p->data);
+        stdvga_outb(port, val);
         break;
 
     case 2:
-        rc &= stdvga_outb(p->addr + 0, p->data >> 0);
-        rc &= stdvga_outb(p->addr + 1, p->data >> 8);
+        stdvga_outb(port + 0, val >> 0);
+        stdvga_outb(port + 1, val >> 8);
         break;
 
     default:
-        rc = 0;
         break;
     }
-
-    return rc;
 }
 
-int stdvga_intercept_pio(ioreq_t *p)
+int stdvga_intercept_pio(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct hvm_hw_stdvga *s = &current->domain->arch.hvm_domain.stdvga;
-    int rc;
 
-    if ( p->data_is_ptr || (p->dir == IOREQ_READ) )
+    if ( dir == IOREQ_READ )
         return 0;
 
     spin_lock(&s->lock);
-    rc = (stdvga_out(p) && hvm_buffered_io_send(p));
+    stdvga_out(port, bytes, *val);
     spin_unlock(&s->lock);
 
-    return rc;
+    return 0; /* propagate to external ioemu */
 }
 
 #define GET_PLANE(data, p) (((data) >> ((p) * 8)) & 0xff)

xen/arch/x86/hvm/vpic.c

@@ -316,61 +316,45 @@ static uint32_t vpic_ioport_read(struct hvm_hw_vpic *vpic, uint32_t addr)
     return vpic->imr;
 }
 
-static int vpic_intercept_pic_io(ioreq_t *p)
+static int vpic_intercept_pic_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct hvm_hw_vpic *vpic;
-    uint32_t data;
 
-    if ( (p->size != 1) || (p->count != 1) )
+    if ( bytes != 1 )
     {
-        gdprintk(XENLOG_WARNING, "PIC_IO bad access size %d\n", (int)p->size);
+        gdprintk(XENLOG_WARNING, "PIC_IO bad access size %d\n", bytes);
         return 1;
     }
 
-    vpic = &current->domain->arch.hvm_domain.vpic[p->addr >> 7];
+    vpic = &current->domain->arch.hvm_domain.vpic[port >> 7];
 
-    if ( p->dir == IOREQ_WRITE )
-    {
-        if ( p->data_is_ptr )
-            (void)hvm_copy_from_guest_phys(&data, p->data, p->size);
-        else
-            data = p->data;
-        vpic_ioport_write(vpic, (uint32_t)p->addr, (uint8_t)data);
-    }
+    if ( dir == IOREQ_WRITE )
+        vpic_ioport_write(vpic, port, (uint8_t)*val);
     else
-    {
-        data = vpic_ioport_read(vpic, (uint32_t)p->addr);
-        if ( p->data_is_ptr )
-            (void)hvm_copy_to_guest_phys(p->data, &data, p->size);
-        else
-            p->data = (u64)data;
-    }
+        *val = (uint8_t)vpic_ioport_read(vpic, port);
 
     return 1;
 }
 
-static int vpic_intercept_elcr_io(ioreq_t *p)
+static int vpic_intercept_elcr_io(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val)
 {
     struct hvm_hw_vpic *vpic;
     uint32_t data;
 
-    if ( (p->size != 1) || (p->count != 1) )
+    if ( bytes != 1 )
     {
-        gdprintk(XENLOG_WARNING, "PIC_IO bad access size %d\n", (int)p->size);
+        gdprintk(XENLOG_WARNING, "PIC_IO bad access size %d\n", bytes);
         return 1;
     }
 
-    vpic = &current->domain->arch.hvm_domain.vpic[p->addr & 1];
+    vpic = &current->domain->arch.hvm_domain.vpic[port & 1];
 
-    if ( p->dir == IOREQ_WRITE )
+    if ( dir == IOREQ_WRITE )
     {
-        if ( p->data_is_ptr )
-            (void)hvm_copy_from_guest_phys(&data, p->data, p->size);
-        else
-            data = p->data;
-
         /* Some IRs are always edge trig. Slave IR is always level trig. */
-        data &= vpic_elcr_mask(vpic);
+        data = *val & vpic_elcr_mask(vpic);
         if ( vpic->is_master )
             data |= 1 << 2;
         vpic->elcr = data;
@@ -378,12 +362,7 @@ static int vpic_intercept_elcr_io(ioreq_t *p)
     else
     {
         /* Reader should not see hardcoded level-triggered slave IR. */
-        data = vpic->elcr & vpic_elcr_mask(vpic);
-
-        if ( p->data_is_ptr )
-            (void)hvm_copy_to_guest_phys(p->data, &data, p->size);
-        else
-            p->data = data;
+        *val = vpic->elcr & vpic_elcr_mask(vpic);
     }
 
     return 1;

xen/include/asm-x86/hvm/io.h

@@ -86,23 +86,26 @@ struct hvm_io_op {
 #define HVM_MMIO                    1
 #define HVM_BUFFERED_IO             2
 
-typedef int (*intercept_action_t)(ioreq_t *);
 typedef unsigned long (*hvm_mmio_read_t)(struct vcpu *v,
                                          unsigned long addr,
                                          unsigned long length);
-
 typedef void (*hvm_mmio_write_t)(struct vcpu *v,
                                unsigned long addr,
                                unsigned long length,
                                unsigned long val);
-
 typedef int (*hvm_mmio_check_t)(struct vcpu *v, unsigned long addr);
 
+typedef int (*portio_action_t)(
+    int dir, uint32_t port, uint32_t bytes, uint32_t *val);
+typedef int (*mmio_action_t)(ioreq_t *);
 struct io_handler {
     int                 type;
     unsigned long       addr;
     unsigned long       size;
-    intercept_action_t  action;
+    union {
+        portio_action_t portio;
+        mmio_action_t   mmio;
+    } action;
 };
 
 struct hvm_io_handler {
@@ -120,7 +123,7 @@ struct hvm_mmio_handler {
 extern int hvm_io_intercept(ioreq_t *p, int type);
 extern int register_io_handler(
     struct domain *d, unsigned long addr, unsigned long size,
-    intercept_action_t action, int type);
+    void *action, int type);
 
 static inline int hvm_portio_intercept(ioreq_t *p)
 {
@@ -137,14 +140,14 @@ extern int hvm_buffered_io_send(ioreq_t *p);
 
 static inline int register_portio_handler(
     struct domain *d, unsigned long addr,
-    unsigned long size, intercept_action_t action)
+    unsigned long size, portio_action_t action)
 {
     return register_io_handler(d, addr, size, action, HVM_PORTIO);
 }
 
 static inline int register_buffered_io_handler(
     struct domain *d, unsigned long addr,
-    unsigned long size, intercept_action_t action)
+    unsigned long size, mmio_action_t action)
 {
     return register_io_handler(d, addr, size, action, HVM_BUFFERED_IO);
 }