Re: [PATCH v4 07/17] x86/hvm: add length to mmio check op

["Jan Beulich" <JBeulich@suse.com>]

>>> On 24.06.15 at 13:24, <paul.durrant@citrix.com> wrote:
> --- a/xen/arch/x86/hvm/hpet.c
> +++ b/xen/arch/x86/hvm/hpet.c
> @@ -498,10 +498,11 @@ static int hpet_write(
>      return X86EMUL_OKAY;
>  }
>  
> -static int hpet_range(struct vcpu *v, unsigned long addr)
> +static int hpet_range(struct vcpu *v, unsigned long addr,
> +                      unsigned long length)
>  {
> -    return ( (addr >= HPET_BASE_ADDRESS) &&
> -             (addr < (HPET_BASE_ADDRESS + HPET_MMAP_SIZE)) );
> +    return (addr >= HPET_BASE_ADDRESS) &&
> +           ((addr + length) < (HPET_BASE_ADDRESS + HPET_MMAP_SIZE));

<=

> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -986,14 +986,16 @@ int hvm_x2apic_msr_write(struct vcpu *v, unsigned int msr, uint64_t msr_content)
>      return vlapic_reg_write(v, offset, (uint32_t)msr_content);
>  }
>  
> -static int vlapic_range(struct vcpu *v, unsigned long addr)
> +static int vlapic_range(struct vcpu *v, unsigned long address,
> +                        unsigned long len)
>  {
>      struct vlapic *vlapic = vcpu_vlapic(v);
> -    unsigned long offset  = addr - vlapic_base_address(vlapic);
> +    unsigned long offset  = address - vlapic_base_address(vlapic);
>  
>      return !vlapic_hw_disabled(vlapic) &&
>             !vlapic_x2apic_mode(vlapic) &&
> -           (offset < PAGE_SIZE);
> +           (address >= vlapic_base_address(vlapic)) &&
> +           ((offset + len) <= PAGE_SIZE);

I'd prefer to stay with checking just offset here, unless you see
anything wrong with that.

> @@ -333,12 +333,15 @@ out:
>      return r;
>  }
>  
> -static int msixtbl_range(struct vcpu *v, unsigned long addr)
> +static int msixtbl_range(struct vcpu *v, unsigned long address,
> +                         unsigned long len)
>  {
> +    struct msixtbl_entry *entry;
>      const struct msi_desc *desc;
>  
>      rcu_read_lock(&msixtbl_rcu_lock);
> -    desc = msixtbl_addr_to_desc(msixtbl_find_entry(v, addr), addr);
> +    entry = msixtbl_find_entry(v, address, len);
> +    desc = msixtbl_addr_to_desc(entry, address);

Again I don't see the need to do more adjustments here than
necessary for your purpose.

> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -35,7 +35,9 @@ typedef int (*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 (*hvm_mmio_check_t)(struct vcpu *v,
> +                                unsigned long addr,
> +                                unsigned long length);

I don't think this really needs to be "long"?

Jan