>>> On 28.04.16 at 13:17, <paul.durr...@citrix.com> wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 28 April 2016 10:50
>> @@ -366,7 +367,22 @@ static int msixtbl_range(struct vcpu *v,
>> ((addr & (PCI_MSIX_ENTRY_SIZE - 1)) ==
>> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET) &&
>> !(data & PCI_MSIX_VECTOR_BITMASK) )
>> + {
>> v->arch.hvm_vcpu.hvm_io.msix_snoop_address = addr;
>> + v->arch.hvm_vcpu.hvm_io.msix_snoop_gpa = 0;
>> + }
>> + }
>> + else if ( (size == 4 || size == 8) && !r->df &&
>> + r->count && r->count <= PCI_MSIX_ENTRY_SIZE / size &&
>> + !((addr + (size * r->count)) & (PCI_MSIX_ENTRY_SIZE - 1))
>> )
>
> That's quite an if statement. Any chance of making it more decipherable? I
I don't see how I could be doing this.
> also think it's worth putting the restrictions you outline in the commit in a
> comment here so that it's clear that the code is not trying to handle all
> corner cases.
Sure. Question is whether by mixing code and comments things
would get better readable (to at least somewhat address your
request above), or whether that instead would make things
worse. Thoughts?
>> + {
>> + BUILD_BUG_ON((PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET + 4) &
>> + (PCI_MSIX_ENTRY_SIZE - 1));
>> +
>> + v->arch.hvm_vcpu.hvm_io.msix_snoop_address =
>> + addr + size * r->count - 4;
>> + v->arch.hvm_vcpu.hvm_io.msix_snoop_gpa =
>> + r->data + size * r->count - 4;
>
> Does there need to be any explicit type promotion here since r->data is
> uint64_t?
No, because both size and r->count did already get bounds
checked to very narrow ranges.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
