From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 7/8] KVM: assigned dev: Introduce io_device for MSI-X MMIO accessing Date: Thu, 21 Oct 2010 11:24:59 +0200 Message-ID: <20101021092459.GA2950@redhat.com> References: <1287563192-29685-1-git-send-email-sheng@linux.intel.com> <1287563192-29685-8-git-send-email-sheng@linux.intel.com> <4CBEBA87.3040107@redhat.com> <201010211446.51904.sheng@linux.intel.com> <4CC00782.8020903@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Sheng Yang , Marcelo Tosatti , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:34723 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752667Ab0JUJba (ORCPT ); Thu, 21 Oct 2010 05:31:30 -0400 Content-Disposition: inline In-Reply-To: <4CC00782.8020903@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Oct 21, 2010 at 11:27:30AM +0200, Avi Kivity wrote: > On 10/21/2010 08:46 AM, Sheng Yang wrote: > >> > + r = -EOPNOTSUPP; > >> > >> If the guest assigned the device to another guest, it allows the nested > >> guest to kill the non-nested guest. Need to exit in a graceful fashion. > > > >Don't understand... It wouldn't result in kill but return to QEmu/userspace. > > What would qemu do on EOPNOTSUPP? It has no way of knowing that > this was triggered by an unsupported msix access. What can it do? > > Best to just ignore the write. > > If you're worried about debugging, we can have a trace_kvm_discard() > tracepoint that logs the address and a type enum field that explains > why an access was discarded. The issue is that the same page is used for mask and entry programming. > >> > + if (addr % PCI_MSIX_ENTRY_SIZE != PCI_MSIX_ENTRY_VECTOR_CTRL) { > >> > + /* Only allow entry modification when entry was masked */ > >> > + if (!entry_masked) { > >> > + printk(KERN_WARNING > >> > + "KVM: guest try to write unmasked MSI-X entry. " > >> > + "addr 0x%llx, len %d, val 0x%lx\n", > >> > + addr, len, new_val); > >> > + r = 0; > >> > >> What does the spec says about this situation? > > > >As Michael pointed out. The spec said the result is "undefined" indeed. > > Ok. Then we should silently discard the write instead of allowing > the guest to flood host dmesg. > > >> > >> > + goto out; > >> > + } > >> > + if (new_val& ~1ul) { > >> > >> Is there a #define for this bit? > > > >Sorry I didn't find it. mask_msi_irq() also use the number 1... Maybe we can add > >one. > > Yes please. > > > -- > error compiling committee.c: too many arguments to function