diff mbox

[v1,04/10] xen/pt: Use xen_host_pci_get_[byte, word, long] instead of xen_host_pci_get_long

Message ID 1435866681-18468-5-git-send-email-konrad.wilk@oracle.com
State New
Headers show

Commit Message

Konrad Rzeszutek Wilk July 2, 2015, 7:51 p.m. UTC
Otherwise we get:

xen_pt_config_reg_init: Offset 0x0004 mismatch! Emulated=0x0000, host=0x2300017, syncing to 0x2300014.
xen_pt_config_reg_init: Error: Offset 0x0004:0x2300014 expands past register size(2)!

which is not surprising. We read the value as an 32-bit (from host),
then operate it as a 16-bit - and the remainder is left unchanged.

We end up writting the value as 16-bit (so 0014) to dev.config
(as we use proper xen_set_host_[byte,word,long] so we don't spill
to other registers) but in XenPTReg->data it is as 32-bit (0x2300014)!

It is harmless as the read/write functions end up using an size mask
and never modify the bits past 16-bit (reg->size is 2).

This patch fixes the warnings by reading the value using the
proper size.

Note that the check for size is still left in-case the developer
sets bits past the reg->size in the ->init routines.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 hw/xen/xen_pt_config_init.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Stefano Stabellini July 17, 2015, 3:59 p.m. UTC | #1
On Thu, 2 Jul 2015, Konrad Rzeszutek Wilk wrote:
> Otherwise we get:
> 
> xen_pt_config_reg_init: Offset 0x0004 mismatch! Emulated=0x0000, host=0x2300017, syncing to 0x2300014.
> xen_pt_config_reg_init: Error: Offset 0x0004:0x2300014 expands past register size(2)!
> 
> which is not surprising. We read the value as an 32-bit (from host),
> then operate it as a 16-bit - and the remainder is left unchanged.
> 
> We end up writting the value as 16-bit (so 0014) to dev.config
             ^ writing


> (as we use proper xen_set_host_[byte,word,long] so we don't spill
> to other registers) but in XenPTReg->data it is as 32-bit (0x2300014)!
> 
> It is harmless as the read/write functions end up using an size mask
> and never modify the bits past 16-bit (reg->size is 2).
> 
> This patch fixes the warnings by reading the value using the
> proper size.
> 
> Note that the check for size is still left in-case the developer
> sets bits past the reg->size in the ->init routines.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  hw/xen/xen_pt_config_init.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
> index 09309ba..e597993 100644
> --- a/hw/xen/xen_pt_config_init.c
> +++ b/hw/xen/xen_pt_config_init.c
> @@ -1876,7 +1876,12 @@ static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
>          offset = reg_grp->base_offset + reg->offset;
>          size_mask = 0xFFFFFFFF >> ((4 - reg->size) << 3);
>  
> -        rc = xen_host_pci_get_long(&s->real_device, offset, &val);
> +        switch (reg->size) {
> +        case 1: rc = xen_host_pci_get_byte(&s->real_device, offset, (uint8_t *)&val);break;
> +        case 2: rc = xen_host_pci_get_word(&s->real_device, offset, (uint16_t *)&val);break;
> +        case 4: rc = xen_host_pci_get_long(&s->real_device, offset, &val);break;
> +        default: assert(1);
> +        }
>          if (rc) {
>              /* Serious issues when we cannot read the host values! */
>              g_free(reg_entry);

Please merge this patch with patch #2/10.
It is best to place the break statements on new lines, to follow QEMU's code style.
diff mbox

Patch

diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 09309ba..e597993 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1876,7 +1876,12 @@  static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
         offset = reg_grp->base_offset + reg->offset;
         size_mask = 0xFFFFFFFF >> ((4 - reg->size) << 3);
 
-        rc = xen_host_pci_get_long(&s->real_device, offset, &val);
+        switch (reg->size) {
+        case 1: rc = xen_host_pci_get_byte(&s->real_device, offset, (uint8_t *)&val);break;
+        case 2: rc = xen_host_pci_get_word(&s->real_device, offset, (uint16_t *)&val);break;
+        case 4: rc = xen_host_pci_get_long(&s->real_device, offset, &val);break;
+        default: assert(1);
+        }
         if (rc) {
             /* Serious issues when we cannot read the host values! */
             g_free(reg_entry);