On Tue, Aug 2, 2016 at 3:16 PM, Jan Kiszka <jan.kis...@siemens.com> wrote:
> On 2016-08-02 13:58, David Kiarie wrote: > > > > > > On Tue, Aug 2, 2016 at 1:28 PM, Peter Xu <pet...@redhat.com > > <mailto:pet...@redhat.com>> wrote: > > > > On Tue, Aug 02, 2016 at 10:46:13AM +0200, Jan Kiszka wrote: > > > On 2016-08-02 10:36, Peter Xu wrote: > > > > On Mon, Aug 01, 2016 at 06:39:05PM +0200, Jan Kiszka wrote: > > > > > > > > [...] > > > > > > > >>> static MemTxResult vtd_mem_ir_read(void *opaque, hwaddr addr, > > > >>> @@ -2209,11 +2250,17 @@ static MemTxResult > > vtd_mem_ir_write(void *opaque, hwaddr addr, > > > >>> { > > > >>> int ret = 0; > > > >>> MSIMessage from = {}, to = {}; > > > >>> + uint16_t sid = X86_IOMMU_SID_INVALID; > > > >>> > > > >>> from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST; > > > >>> from.data = (uint32_t) value; > > > >>> > > > >>> - ret = vtd_interrupt_remap_msi(opaque, &from, &to); > > > >>> + if (!attrs.unspecified) { > > > >>> + /* We have explicit Source ID */ > > > >>> + sid = attrs.requester_id; > > > >>> + } > > > >> > > > >> ...here you fall back to X86_IOMMU_SID_INVALID if writer to > > this region > > > >> has not provided some valid attrs. That is questionable, defeats > > > >> validation of the IOAPIC e.g. (and you can see lots of > > > >> X86_IOMMU_SID_INVALID in vtd_irte_get when booting a guest). > > > >> > > > >> The credits also go to David who noticed that he still doesn't > > get a > > > >> proper ID from the IOAPIC while implementing AMD IR. Looks like > > we need > > > >> to enlighten the IOAPIC MSI writes... > > > > > > > > Jan, David, > > > > > > > > At the time when drafting the patch, I skipped SID verification > for > > > > IOAPIC interrupts since it differs from generic PCI devices (no > > > > natural requester ID, so need some hacky lines to enable it). > > > > > > It's not hacky at all if done properly. For Intel it is simply > > > (Q35_PSEUDO_BUS_PLATFORM << 8) | Q35_PSEUDO_DEVFN_IOAPIC, but it > > will be > > > 0x00a0 (as constant as well) for AMD. So we need some interface to > > tell > > > those parameters to the IOMMU. Keep in mind that we will need a > > similar > > > interface for other platform devices, e.g. the HPET. > > > > Okay. > > > > > > > > > > > > > I can try to cook another seperate patch to enable it (for 2.8 > > > > possibly?). Thanks for pointing out this issue. > > > > > > David needs that IOAPIC ID as well in order to finish interrupt > > > remapping on AMD. Please synchronize with him who will implement > what. > > > > Sure. David, so do you like to do it or I cook this patch? :) > > > > > > If there are no objections I will look at this employing Jan's approach: > > associating a write with an address space. > > > > That actually means making it an IOMMU-local thing (if write_requester > == invalid -> derive ID from addressed memory region, probably via the > opaque passed to the ops). In that case, everyone could do this on himself. > Yes, each person can actually do their part without affecting the other ;-) > > Jan > > -- > Siemens AG, Corporate Technology, CT RDA ITP SES-DE > Corporate Competence Center Embedded Linux >
