On Mon, Jul 31, 2023 at 2:20 PM Christopher Clark
<[email protected]> wrote:
>
>
>
> On Fri, Jul 28, 2023 at 5:52 AM Bruce Ashfield <[email protected]> 
> wrote:
>>
>> I'm travelling today, so I can't formulate a full answer at the moment.
>>
>> I just wanted to let everyone know that I have been reading and do
>> have an opinion which I'll send out when I get a chance to properly
>> write an email.
>>
>> I hadn't seen the patches in great detail before this send, so I need
>> to go through things and have a closer look.
>>
>> Generally speaking, I support having related recipes in a layer, as a
>> full reference stack.  Perhaps there's a subset of things that are too
>> hardware/board specific, but again, I haven't looked in detail yet.
>> Layer "expansion" is also an issue for maintenance in the overall
>> ecosystem. Having too many layers is an issue for maintenance, along
>> with having too few layers that don't have a clean split between
>> distro/machine and functionality.
>>
>>
>> Even if some of this does land in a non-xilinx specific layer (that
>> isn't meta-virt), we'd be well served to have a README or other file
>> in meta-virt that explains how to build for hardware platforms. Having
>> only qemu available in meta-virt, and then having integrators wander
>> off into the wild world of non peer reviewed vendor layers and forks
>> of various components is something that I've been concerned about for
>> quite some time.
>
>
> Thanks for replying -- the challenges of dealing with layer proliferation are 
> real, which is part of what motivated the introduction of the dynamic layer 
> for the Raspberry Pi board in support of enabling simpler testing, and I 
> hadn't given that enough consideration in my response earlier. Without a 
> reference document or direct example, it's not easy - it takes practical 
> experience and persistence with it at the moment, so thanks for doing that 
> work, Sandeep, and I apologise. The dynamic layers are good for enabling 
> compatibility with composition and these changes are in support of a positive 
> experience with enabling Xen platforms, so please continue.
>

I'm finally back at a keyboard after 20+ hours of driving in the past 4 days.

I won't dig in much more to the patches, but I wanted to be clear that
I wasn't criticizing your comment in the slightest. We need that sort
of review.

If we do find something that is inappropriate that I miss in the
patches, please flag it and I'll remove it and ask for it to go to a
BSP layer.

Cheers,

Bruce

> best,
>
> Christopher
>
>>
>>
>> That all being said, don't take any of the above as more than my
>> initial thoughts. I'm extremely pleased to see Christophers review and
>> questions .. that's exactly what we need!
>>
>> Bruce
>>
>>
>>
>> On Fri, Jul 28, 2023 at 2:06 AM Christopher Clark
>> <[email protected]> wrote:
>> >
>> > +CC Bertrand
>> >
>> > On Thu, Jul 27, 2023 at 5:38 PM Sandeep Gundlupet Raju via 
>> > lists.yoctoproject.org 
>> > <[email protected]> wrote:
>> >>
>> >> Hi Chris,
>> >>
>> >> On 7/27/2023 6:10 PM, Christopher Clark via lists.yoctoproject.org wrote:
>> >>
>> >>
>> >> On Thu, Jul 27, 2023 at 4:22 PM Sandeep Gundlupet Raju 
>> >> <[email protected]> wrote:
>> >>>
>> >>> Add Xen dt and u-boot script support for AMD ZynqMP and Versal devices.
>> >>
>> >>
>> >> Hi Sandeep - this is a slightly surprising submission. I'm unsure about 
>> >> meta-virtualization being the right place for this as typically hardware 
>> >> enablement is more appropriately done in a BSP layer, where engineers who 
>> >> have knowledge about and access to specific hardware can collaborate on 
>> >> maintaining the machine-specific aspects of recipes.
>> >>
>> >> [Sandeep]: I checked with Bruce before I submit the patches, Since we had 
>> >> xilinx dynamic layer, he has no objection in accepting the patches.
>> >
>> >
>> > That is generous, but I think you could make the layer maintenance 
>> > responsibilities easier and help with establishing good practice for 
>> > hardware support if these changes can go into a BSP layer instead, if that 
>> > is an option? I do appreciate that you are working on enabling Xen to work 
>> > on additional platforms.
>> >
>> > The Xilinx dynamic layer could be removed from meta-virtualization by 
>> > adding its Xen image creation logic to the primary Xen hypervisor recipe 
>> > -- possibly with a new mkimage task? --  to be enabled via new settings in 
>> > an Arm or Xilinx MACHINE configuration.
>> >
>> >>
>> >> Xen in this (non-BSP) layer has support for qemu MACHINES, plus generic 
>> >> x86-64 and the Raspberry Pi 4 board for Arm platform coverage; the latter 
>> >> is a special-case that has been discussed by Xen and Yocto community 
>> >> members as a basic commodity reference hardware platform for testing (and 
>> >> using) Xen, and the community for this layer has more of the interested 
>> >> parties participating in it than meta-raspberrypi does.
>> >>
>> >> Are you experiencing difficulties with maintaining your Xen recipe 
>> >> components in a separate layer?
>> >>
>> >> [Sandeep]: No we are not experiencing any difficulties in maintaining 
>> >> these files, currently all the dtsi files are in meta-petalinux layer and 
>> >> if users are using without meta-petalinux distro layer(building 
>> >> xen-image-minimal), then Xen doesn't work on ZynqMP and Versal devices.
>> >
>> >
>> > Do you know which layer defines the MACHINE configurations for those 
>> > hardware devices? It seems like that could be a suitable BSP layer for 
>> > these changes?
>> >
>> >>
>> >> Mark Hatle and myself we discussed on this and we saw rpi u-boot script 
>> >> and other files in this layer, we thought this is a good place to move 
>> >> these files.
>> >
>> >
>> > I see how the raspberry pi dynamic layer can give that impression which is 
>> > unfortunate. The motivation for the inclusion of the rpi files in 
>> > meta-virtualization is to enable collaboration on hardware testing (and 
>> > reproduction and diagnosis if needed) of the Arm architecture 
>> > functionality of the primary Xen recipes, on a low-cost commodity hardware 
>> > board, with agreement on this from Xen stakeholders at multiple different 
>> > organizations. X86 doesn't need it as generic x86-64 support (see 
>> > meta-yocto-bsp) is enough.
>> >
>> > Christopher
>> >
>> >
>> >>
>> >> Mark can chime in here to add any additional comments.
>> >>
>> >>
>> >> Christopher
>> >>
>> >>>
>> >>>
>> >>> User can enable xen u-boot script by adding below variable from
>> >>> configuration file as shown below.
>> >>>
>> >>> BOOTMODE = "xen"
>> >>>
>> >>> This u-boot script also supports to configure below Xen params from
>> >>> configuration file.
>> >>>
>> >>> params          variable assignment
>> >>> ------          -------------------
>> >>> duart           XEN_SERIAL_CONSOLES = "serial0"
>> >>> dom0_mem        DOM0_MEM = "1500M"
>> >>> dom0_max_vcpus  DOM0_MAX_VCPUS = "1"
>> >>> extra params    XEN_CMDLINE_APPEND = "loglvl=all"
>> >>>
>> >>> Sandeep Gundlupet Raju (2):
>> >>>   recipes-bsp: Add device-tree files for Xen support
>> >>>   recipes-bsp: Add u-boot-xlnx scripts for Xen support
>> >>>
>> >>>  .../device-tree/device-tree.bbappend          |  19 +++
>> >>>  .../files/versal-net-xen-qemu.dtsi            |   2 +
>> >>>  .../device-tree/files/versal-net-xen.dtsi     |  67 ++++++++++
>> >>>  .../device-tree/files/versal-xen-qemu.dtsi    |   2 +
>> >>>  .../device-tree/files/versal-xen.dtsi         |  59 +++++++++
>> >>>  .../device-tree/files/zynqmp-xen-qemu.dtsi    |  16 +++
>> >>>  .../device-tree/files/zynqmp-xen.dtsi         | 123 ++++++++++++++++++
>> >>>  .../u-boot/u-boot-xlnx-scr.bbappend           |   2 +
>> >>>  .../u-boot/u-boot-xlnx-scr/boot.cmd.xen       |  80 ++++++++++++
>> >>>  .../recipes-bsp/u-boot/xen-boot-cmd.inc       |  38 ++++++
>> >>>  10 files changed, 408 insertions(+)
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/device-tree/device-tree.bbappend
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/device-tree/files/versal-net-xen-qemu.dtsi
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/device-tree/files/versal-net-xen.dtsi
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/device-tree/files/versal-xen-qemu.dtsi
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/device-tree/files/versal-xen.dtsi
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/device-tree/files/zynqmp-xen-qemu.dtsi
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/device-tree/files/zynqmp-xen.dtsi
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/u-boot/u-boot-xlnx-scr.bbappend
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/u-boot/u-boot-xlnx-scr/boot.cmd.xen
>> >>>  create mode 100644 
>> >>> dynamic-layers/xilinx/recipes-bsp/u-boot/xen-boot-cmd.inc
>> >>>
>> >>> --
>> >>> 2.34.1
>> >>>
>> >>
>> >>
>> >> 
>> >>
>>
>>
>> --
>> - Thou shalt not follow the NULL pointer, for chaos and madness await
>> thee at its end
>> - "Use the force Harry" - Gandalf, Star Trek II



-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#8158): 
https://lists.yoctoproject.org/g/meta-virtualization/message/8158
Mute This Topic: https://lists.yoctoproject.org/mt/100401326/21656
Group Owner: [email protected]
Unsubscribe: 
https://lists.yoctoproject.org/g/meta-virtualization/leave/6693005/21656/1014668956/xyzzy
 [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to