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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html