On 08/14/2012 08:23 PM, Alex Williamson wrote:
>
>> Unrelated nit: memcmp() doesn't return a boolean or a count, so
>> !memcmp() is really unintuitive, at least to me.
>
> I figure we're all pretty used to it growing up on !strcmp though.
I hate that one too.
>> > +
>> > +/* XXX This should move to msi.c */
>>
>> Well?
>
> Just marking a todo item. I'll change it formally to TODO. I think
> there are a few interfaces to msi.c that probably needs some rethinking
> for device assignment. When they're small like this it seems easier to
> have the user in tree first.
I prefer them in the right place but I don't insist.
>> > +
>> > + if (unlikely((section->offset_within_address_space &
>> > ~TARGET_PAGE_MASK) !=
>> > + (section->offset_within_region & ~TARGET_PAGE_MASK))) {
>> > + error_report("%s received unaligned region\n", __func__);
>>
>> Is it really an error? I think you can just add the condition to
>> skipped_section.
>
> I had left this in as paranoia for myself that I wanted to see if this
> actually happens. I want to assume that our TARGET_PAGE_ALIGNED
> offset_within_address_space results in an aligned ram pointer. If one
> is aligned different from the other we're kinda screwed trying to map it
> into the iommu. So far I haven't seen it. Thanks for the feedback,
We could have a sub-page RAM region (perhaps inserted as a mapped BAR
from some emulated device, or from vfio if/when it grows that capability).
But you're right, it really is an error, we can't just ignore it. So
the current code is right.
--
error compiling committee.c: too many arguments to function
