[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH+RFC+HACK 00/16] xen: arm initial support for xgene arm64 platform



On Wed, Nov 20, 2013 at 2:45 PM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> I'm afraid this series is rather a grab bag and it is distressingly
> large at this stage. With this series I can boot an Xgene board until it
> fails to find its SATA controller. This is a dom0 issue for which
> patches are pending from APM (/me nudges Anup).
>
> As well as the APM specific platform stuff there are also some generic
> improvements which were either necessary or useful during this work.
> Towards the tail end are some hacks and rfcs which need more work and/or
> discussion. I mostly posting now because I'm aware that I've been
> negligent in not sending these out sooner.
>
> WRT the freeze I think that the baseline stuff is all plausible for 4.4.
> Firstly because I'm inclined to give new platform enablement a fairly
> loose reign at least for the time being (and much of it was posted ages
> ago by Anup/Pranavkumar). Secondly the non-platform related bits (other
> than the aforementioned hacks/rfcs) fall mostly either into two buckets:
> Either they are bugfixes or they are mostly obviously safe (additional
> debug prints etc).
>
> Blow by blow analysis:
>
>         xen: arm64: Add 8250 earlyprintk support
>
>                 New early uart driver. It is enabled as a build time
>                 debug option and is totally harmless to platforms which
>                 don't use it.
>
>         xen: arm64: Add Basic Platform support for APM X-Gene Storm.
>         xen: arm64: Add APM implementor id to processor implementers.
>         xen: arm: include ns16550 driver on arm64 too
>         xen: arm: Enable 1:1 workaround for APM X-Gene Storm.
>
>                 Support for the new platform. Enable an existing driver
>                 used by that platform (already on for arm32).
>
>         xen: arm: early logging of command line
>
>                 Pretty safe & very useful IMNSVHO.

All of these look fine from a release perspective.

>
>         xen: arm: Handle cpus nodes with #address-calls > 1

So we need to make a distinction here with "bug fixes": from a release
perspective, bugs are errors in the code that affect working features.
 What is the likelihood that any currently-supported architectures
might problems without this patch?  It's not clear from looking at the
patch.  If it's low-to-none, then this wouldn't really qualify for a
bug fix exemption to the code freeze.

>         xen: arm: Make register bit definitions unsigned.
>         xen: arm: explicitly map 64 bit release address
>
>                 Bug fixes.

These two look fine.

>
>         xen: arm: enable synchronous console while starting secondary CPUs
>
>                 Improves logging in a useful way. Pretty safe.
>
>         xen: arm: Add debug keyhandler to dump the physical GIC state.
>
>                 Useful debug functionality. Harmless unless you
>                 deliberately trigger the particular debug key.
>
>         xen: arm: improve early memory map readability
>
>                 Cosmetic, but safe.

These all look fine as well.

>
>         RFC: xen: arm: handle 40-bit addresses in the p2m
>         RFC: xen: arm: allow platform code to select dom0 event channel irq
>
>                 These should be considered for cleanup review and
>                 eventual commit for 4.4. The rest of the platform
>                 enablement is pretty pointless without these.

Hmm... it seems a bit late for RFC stuff in fairly core code.  These
look like they might possibly be extending functionality for
currently-supported architectures as well; but without concrete
examples, this would come under "new feature" rather than "bug fix".

On the other hand, both of these look pretty obvious and low-risk improvements.

>
>         HACK: xen: arm: GICC_DIR register at offset 0x10000 instead of 0x1000
>
>                 Should be properly implemented with a view to being
>                 accepted for 4.4. Again things are rather pointless
>                 without.
>
>                 Could plausibly be reimplemented as a platform quirk,
>                 which might be safer for 4.4.
>
>         HACK: xen: arm: map PCI controller ranges region MMIOs to dom0.
>
>                 I think this one is likely to be a step too far for 4.4.
>                 Even if it worked (it doesn't) it is quite a big and
>                 potentially complex change. I'm considering the option
>                 of implementing the hardcoded list (which is here as a
>                 HACK, see the commit message for more) via the
>                 platform->specific_mapping callback for 4.4. In that
>                 case it would only impact Xgene if it were broken.

This is starting to look like an awful lot of "to be sorted out".
(Although it looks like Julien has a simple solution that makes this
last patch unnecessary?)

You address risks, but you don't address the fundamental benefit of
including it now, rather than waiting to check it in for 4.5.  At the
moment, unless there is some compelling strategic reason for including
this in 4.4, I'm inclined to say it should just wait.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.