From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 04/10] xen/arm: vgic-v3: Don't check the size when we ignore the write/read as zero Date: Tue, 20 Jan 2015 17:41:21 +0000 Message-ID: <54BE9341.7010307@linaro.org> References: <1421684957-29884-1-git-send-email-julien.grall@linaro.org> <1421684957-29884-5-git-send-email-julien.grall@linaro.org> <1421769455.10440.306.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YDcoJ-0006Kk-W1 for xen-devel@lists.xenproject.org; Tue, 20 Jan 2015 17:41:52 +0000 Received: by mail-wi0-f171.google.com with SMTP id l15so18022297wiw.4 for ; Tue, 20 Jan 2015 09:41:50 -0800 (PST) In-Reply-To: <1421769455.10440.306.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 20/01/15 15:57, Ian Campbell wrote: > On Mon, 2015-01-19 at 16:29 +0000, Julien Grall wrote: >> In general, it's not necessary/important to check the size. > > Only if the docs say this register can be accessed by a partial > read/write, or if it is implementation defined what the result would be > (and RAZ/WI is within the set of allowable actions). > > Do you have a reference for the behaviour of GICR accesses which aren't > of the register's natural size? It's clearly specify in the spec if the register can be accessed with a non-natural size. AFAICT, the spec doesn't give a specific behavior if the register doesn't support byte/word/double word access. IHMO, for RAZ register we can safely accept any kind of access as the final register will in fine always contains 0. That will also any issue with WI/RAZ register because we may have miss a line in the spec stating a register is byte and word accessible. (see the case of vgic-v2). >> It's better >> to log it to let know the guest that its access will have no effect. >> >> Note: On debug build it may happen to see some of these messages during >> domain boot. > > We should only print if the guest has done something wrong, and reading > a RAZ register (or one which we have implemented that way) is not > inherently wrong. That's why it's log in DEBUG and not in ERR. Although, I agree that on read it's not important. But on write it's help the developer to understand which his GIC driver is not working. > IOW read_as_zero* should be silent, and a different code path used for > "guest did something wrong". > > IOW I think the current distinction between bad_width and read_as_zero* > is correct currently, although perhaps the goto's which target them need > adjusting in some cases. > > Perhaps you want to add a read_as_zero_32 which has the check, making > read_as_zero accept either 32- or 64-bit accesses, and goto the correct > label for each register? It's register defined which access is allowed for a register. Regards, -- Julien Grall