From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 7/8] KVM: assigned dev: Introduce io_device for MSI-X MMIO accessing Date: Thu, 21 Oct 2010 11:27:30 +0200 Message-ID: <4CC00782.8020903@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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org, "Michael S. Tsirkin" To: Sheng Yang Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42155 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756034Ab0JUJ1f (ORCPT ); Thu, 21 Oct 2010 05:27:35 -0400 In-Reply-To: <201010211446.51904.sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: 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. > > > + 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