Re: [PATCH v4 5/8] hvmloader: Correct bug in low mmio region accounting

[George Dunlap <george.dunlap@eu.citrix.com>]

On 21/06/13 14:32, Ian Jackson wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH v4 5/8] hvmloader: Correct bug in low mmio region accounting"):
>> On 21.06.13 at 13:19, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>>> But for the first device I think it may be possible for resource->base
>>> not to be a multiple of the bar_sz, and in that case it might be that
>>> the precalculation thinks it will fit when the actual placement
>>> calculation doesn't.
>>>
>>> Do you think this is possible ?
>> This is possible only from an abstract perspective, not in reality:
>> PCI_MEM_START being 0x{f,e,c,8}0000000, PCI_MEM_END being
>> 0xfc000000, and allocations starting with the biggest BARs
>> (where you already correctly noted that BARs are always a power
>> of 2 in size), the current base address can be misaligned only
>> when the BAR size is too large to fit anyway. In which case it'll
>> go into the space above 4Gb, and to that range the precalculation
>> doesn't apply.
> Ah.  Right.  Err, OK.  I'm convinced by this argument.
>
> It's not a good reflection on the clarity of this code, though.
> Perhaps, George, you could mention this issue in a comment or the
> commit message.

Yes, I think I shall.  It is, as Jan says, correct at the present 
moment, but it's not even clear whether that was by accident or by 
design; even if it was by design, there's no guarantee it will remain so 
in the future without at least a comment.

We may want to try to clean this up long-term, but I would really like 
to investigate just punting this whole thing off to SeaBIOS, which is 
being tested and maintained by the KVM folks.

> But anyway, this, and 6/8,
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Great, thanks.

Stefano pointed out some "development process" terminology leaking into 
the comment on the last patch -- I'll clean that up, add in some 
comments about the fragile accounting, and send v5.  That should be it 
for this series, I think.

  -George