> From: Peter Xu [mailto:pet...@redhat.com] > Sent: Friday, January 20, 2017 5:28 PM > > On Fri, Jan 20, 2017 at 09:15:27AM +0000, Tian, Kevin wrote: > > > From: Peter Xu [mailto:pet...@redhat.com] > > > Sent: Friday, January 20, 2017 5:05 PM > > > > > > On Fri, Jan 20, 2017 at 08:22:14AM +0000, Tian, Kevin wrote: > > > > > From: Peter Xu [mailto:pet...@redhat.com] > > > > > Sent: Friday, January 13, 2017 11:06 AM > > > > > > > > > > Before we have int-remap, we need to bypass interrupt write requests. > > > > > That's not necessary now - we have supported int-remap, and all the > > > > > irq > > > > > region requests should be redirected there. Cleaning up the block with > > > > > an assertion instead. > > > > > > > > This comment is not accurate. According to code, the reason why you > > > > can do such simplification is because we have standalone memory > > > > region now for interrupt addresses. There should be nothing to do > > > > with int-remap, which can be disabled by guest... Maybe the standalone > > > > region was added when developing int-remap, but functionally they > > > > are not related. :-) > > > > > > IMHO the above commit message is fairly clear. :-) > > > > > > But sure I can add some more emphasise like: > > > > > > "Before we have int-remap memory region, ..." > > > > > > Do you think it's okay? Or any better suggestion? > > > > > > (Just to mention that even guest disables IR, the MSI region will > > > still be there.) > > > > > > > My option is simple - this patch has nothing to do with int-remap. > > It's not necessary, not because we supported int-remap. It's because > > we have a standalone memory region for interrupt addresses, as you > > described in the code. :-) > > I really think they are the same thing... > > How about this: > > Now we have a standalone memory region for MSI, all the irq region > requests should be redirected there. Cleaning up the block with an > assertion instead. >
btw what about guest setups a valid mapping at 0xFEEx_xxxx in its remapping structure, which is then programmed to virtual device as DMA destination? Then when emulating that virtual DMA, vtd_do_iommu_translate should simply return (maybe throw out a warning for diagnostic purpose) instead of assert here. VT-d spec defines as below: Software must ensure the second-level paging-structure entries are programmed not to remap input addresses to the interrupt address range. Hardware behavior is undefined for memory requests remapped to the interrupt address range. I don't think "hardware behavior is undefined" is equal to "assert thus kill VM"... Thanks Kevin