[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>

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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.