Re: [Xen-devel] [PATCH v1 04/10] xen/pt: Use xen_host_pci_get_[byte, word, long] instead of xen_host_pci_get_long

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
>  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.

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