From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53541) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzSjb-0001cy-5F for qemu-devel@nongnu.org; Wed, 17 Jul 2013 10:29:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UzSjX-0003LX-1J for qemu-devel@nongnu.org; Wed, 17 Jul 2013 10:29:39 -0400 Received: from mail-qa0-x230.google.com ([2607:f8b0:400d:c00::230]:59394) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UzSjW-0003LR-TX for qemu-devel@nongnu.org; Wed, 17 Jul 2013 10:29:34 -0400 Received: by mail-qa0-f48.google.com with SMTP id cm16so1142247qab.0 for ; Wed, 17 Jul 2013 07:29:34 -0700 (PDT) Sender: Richard Henderson Message-ID: <51E6AA4A.9020101@twiddle.net> Date: Wed, 17 Jul 2013 07:29:30 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1373840171-25556-1-git-send-email-rth@twiddle.net> <1373840171-25556-4-git-send-email-rth@twiddle.net> <8761w9wm50.fsf@blackfin.pond.sub.org> <51E67B7A.8000800@redhat.com> <51E69AE1.1060809@twiddle.net> <51E6A003.8070106@redhat.com> In-Reply-To: <51E6A003.8070106@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL 3/5] exec: Support 64-bit operations in address_space_rw List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aliguori@us.ibm.com, Gerd Hoffmann , Markus Armbruster , qemu-devel@nongnu.org On 07/17/2013 06:45 AM, Paolo Bonzini wrote: >> NAK. >> >> If you remove the check here, you're just trading it for one in the device. >> The device told you that it can't support a 1 byte read. (Either that, or the >> device incorrectly reported what it can actually do.) > > There are two parts to this. > > First of all, mr->ops->impl.min_access_size is definitely wrong. The > device told me that the MMIO functions only know about 2-byte accesses, > but that it _can_ support 1-, 2- and 4- byte reads (with coalescing done > by memory.c). I don't know enough about the specific device (or even which device it was) to know whether the IMPL and VALID fields are correct. > So I could change access_size_min to > mr->ops->valid.min_access_size, which would also fix Markus's problem. No, you can't. At least not without changing all of the callers. If you do as you suggest, the callers will invoke the device with a value of SIZE that is illegal according to IMPL. We might as well crash now than later. There are three possible solutions: (1) Return an error from memory_access_size, change the callers to propagate the error in some fashion. This isn't ideal, since in this case VALID indicates that the guest access is correct. (2) Return the implementation minimum, change the callers to interact with the device using that minimum. With this scenario, we should likely share code with access_with_adjusted_size. (3) Determine that the device's impl.min_access_size is wrong and adjust it. Responding to your earlier > erroneous accesses must not crash > QEMU, they should trigger exceptions in the guest or just return > garbage (depending on the CPU). I completely agree -- if we were talking about VALID. Since this is IMPL, it's not an "erroneous access", but rather QEMU not being self-consistent. And for internal logic errors, we've got asserts and aborts all over. r~