On Tue, Jun 07, 2016 at 01:23:35PM +0200, Alexander Gordeev wrote: > On Tue, Jun 07, 2016 at 12:33:54PM +0200, Alexander Gordeev wrote: > > > > phys_addr_t mask = (int32_t)pci_bar_mask(bar); > > > > > > It might be OK to do that here, on a mask, but even if it is, then I > > > don't like it, because it's too subtle (like I said for the casting > > > in pci_bar_addr in my last reply). I don't like that it requires us to > > > know that masking bit 31 in a 32-bit mask means we also want to mask > > > 63..32. That should at least be in a comment somewhere. > > > > Yes, but it is not an arbitrary mask, it is an alignment mask. > > We unconditionally want to mask 63..32, 31th and even lower. > > Seems like making pci_bar_mask() return phys_addr_t rather than > uint32_t would make more sense. See last reply. If a BAR mask is suppose to operate on a BAR (4 bytes) then it should be 32-bits and not applied to a 64-bit operation. And, as the upper 32-bits of a 64-bit bar don't need any masking anyway, then there's no point in complicating things by attempting to mask it. Thanks, drew -- 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
