From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=36858 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q6kzE-0001m7-Ok for qemu-devel@nongnu.org; Mon, 04 Apr 2011 10:43:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q6kz6-0005ET-LX for qemu-devel@nongnu.org; Mon, 04 Apr 2011 10:42:30 -0400 Received: from mail-vw0-f45.google.com ([209.85.212.45]:43184) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q6kz6-0005EM-Ia for qemu-devel@nongnu.org; Mon, 04 Apr 2011 10:42:28 -0400 Received: by vws17 with SMTP id 17so4818879vws.4 for ; Mon, 04 Apr 2011 07:42:28 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4D99D5BA.3010403@redhat.com> References: <1301407704-24541-1-git-send-email-peter.maydell@linaro.org> <4D92E057.4010005@redhat.com> <4D930B2A.4080808@redhat.com> <4D933677.8000606@codemonkey.ws> <4D99D5BA.3010403@redhat.com> Date: Mon, 4 Apr 2011 15:42:27 +0100 Message-ID: Subject: Re: [Qemu-devel] [PATCH v3 0/7] Let boards state maximum RAM limits in QEMUMachine struct From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: Blue Swirl , qemu-devel@nongnu.org, patches@linaro.org On 4 April 2011 15:29, Jes Sorensen wrote: > On 03/30/11 16:07, Peter Maydell wrote: >> On 30 March 2011 14:56, Anthony Liguori wrote: >>> On 03/30/2011 08:22 AM, Peter Maydell wrote: >>>> Not really, typically they're just filled up in some particular >>>> order (main RAM in one place and expansion RAM elsewhere). >>>> Since the board init function is only passed a single "ram_size" >>>> parameter that's all you can do anyhow. >>> >>> FWIW, I don't think any static data is going to be perfect here. =C2=A0= A lot of >>> boards have strict requirements on ram_size based on plausible combinat= ions >>> of DIMMs. =C2=A0Arbitrary values up to ram_size is not good enough. >>> >>> ram_size ought to be viewed as a hint, not a mechanism to allow common = code >>> to completely validate the passed in ram size parameter. =C2=A0It's sti= ll up to >>> the board to validate that the given ram size makes sense. >> >> Yes, I agree, so we shouldn't try to specify some complicated >> set of static data that still won't be good enough. >> >> I'm trying to make it easy for boards to avoid crashing horribly >> when the user passes a bad value; that's all. > > If you don't validate properly, is there really a point in introducing > that value anyway? From what you write, it sounds like it can still fail > for some limits of the memory valid if the config is wrong? For the boards I care about (the ARM ones), the only validation requirement is that we don't allow the user to specify so much ram that we overlap physical RAM with I/O space. So ram_size is good enough. For the sun4m boards we can assume that the only validation they need is a ram_size check, because that's all they do at the moment and nobody's complaining that I know of. As far as I know Anthony is suggesting that some boards might in theory have stricter memory size requirements. I agree that this is a possibility, but if you have a rare board which has an idiosyncratic requirement then the correct way to handle that is to add extra checks in the board init functions. I don't see a huge queue of people with patches to add complex checks to board init functions that would indicate that we need a generic mechanism for handling this. What we do have is simple ram_size limit checks (in sun4m and we need them for the arm devboards) -- which is a demonstrated need that justifies the common code framework. > It still seems to me it would be better to have the boards present a > table of valid memory ranges so we can do a proper validation of the valu= d? If you have a concrete example of multiple boards which we currently model and which require this level of flexibility to avoid odd misbehaviour tryin= g to run a guest, then please point them out and I'll look at expanding the patch to cover their requirements. If this is just a theoretical issue, then I think we should only add the extra generic framework code if and when we turn out to need it. -- PMM