Skip to content

Conversation

orzelmichal
Copy link
Contributor

This series adds arm support into XTF and creates a base for further implementation.
Support for arm64 allows to run a hello-world test with 2 environments:
-64le: arm64, little endian, no MMU
-mmu64le: arm64, little endian, MMU
Support for arm32 allows to run only a startup code.

This series was tested using qemu-system-aarch64 and FVP_Base_RevC-2xAEMvA with XTF running
as dom0 or domU.

Please review, comment, ask questions.

Instructions on how to prepare base setup for testing XTF on arm64 using QEMU:

  1. Build XTF:
    make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CONFIG_LOAD_ADDRESS=0x80000000
    -this will result in 2 binaries inside tests/example:
    ->test-arm-64le-example - no MMU(uses 0x80000000 as a start address)
    ->test-arm-mmu64le-example - with MMU

  2. Build Xen for arm64
    -select early printk for PL011, 0x9000000, 115200

  3. Generate device tree:
    qemu-system-aarch64 -machine virt,gic_version=3 -machine virtualization=true -cpu cortex-a57 -machine type=virt -m 4096 -smp 4 -display none -machine dumpdtb=virt-gicv3.dtb

  4. Add chosen node:
    chosen {
    xen,xen-bootargs = "noreboot dom0_mem=1024M console=dtuart dtuart=serial0 bootscrub=0 iommu=no";

modules {
#size-cells = <0x1>;
#address-cells = <0x1>;

module@0 {
	reg = <0x47000000 0x1000000>;
	compatible = "xen,linux-zimage", "multiboot,module";
};

};
};

  1. Build u-boot for qemu arm64

  2. Run Xen on QEMU with XTF as dom0:
    qemu-system-aarch64
    -machine virt,gic_version=3
    -machine virtualization=true
    -cpu cortex-a57
    -machine type=virt -m 4096 -smp 4
    -bios <u_boot_binary>
    -device loader,file=<xen_binary>,force-raw=on,addr=0x49000000
    -device loader,file=test-arm-mmu64le-example,addr=0x47000000
    -device loader,file=virt-gicv3.dtb,addr=0x44000000
    -nographic -no-reboot
    -chardev socket,id=qemu-monitor,host=localhost,port=7777,server,nowait,telnet
    -mon qemu-monitor,mode=readline

Currently XTF build system is strictly made up for x86.
Modify the build system so that it will be easier
to add other architectures to XTF in the future.
This is done by generalizing the common makefiles to be
architecture independent and creating architecture
specific makefiles.

Introduce new variable SUBARCH which is the name of
the architecture devoid of information about the bitness,
that expands as follows:
-on x86: x86
-in the future on arm64/arm32: arm

Modify the framework so that the test name contains
the SUBARCH name e.g.:
test-x86-pv64-example
instead of:
test-pv64-example

This change will require a modification in OSSTEST
due to the change in test naming.

Signed-off-by: Michal Orzel <[email protected]>
Add initial support to XTF for arm64/arm32 without
modifying existing XTF architecture.
Some of the files are just dummy files waiting to be properly
implemented step by step later on.

The purpose of this change is to add the initial code and minimal
set of files to support XTF on arm without modifying the existing
XTF design. This creates a base for further implementation.

Based-on-the-work-from: Julien Grall <[email protected]>
Signed-off-by: Michal Orzel <[email protected]>
Add support for arm64/arm32 in the build system,
modify the common headers to be aware of the
new architectures and update the documentation.

Architecture can be set using:
ARCH=arm64 or ARCH=arm32.

Currently only one test: tests/example
is supported allowing the startup code head.S
to be executed ending up in an infinite loop.

Signed-off-by: Michal Orzel <[email protected]>
Each architecture needs to define its own hypercall
handling interface. The hypercall handler should
be named as follows:
hypercall_<hypercall_name>
e.g. hypercall_console_io

The reason for that is to have a standard way
of calling hypercalls in the common code.

Modify the common <xtf/hypercall.h> header to
be architecture independent and move x86 handlers
to architecture specific code.

Signed-off-by: Michal Orzel <[email protected]>
Add the hypercall handlers for arm64 and arm32.
The hypercalls are defined in the assembly files hypercall.S
and they are specific to arm64/arm32.

The prototypes and higher level wrappers are defined
in arch/arm/include/arch/hypercall.h as they are common
to both arm64 and arm32.

Signed-off-by: Michal Orzel <[email protected]>
1) Improve the startup code for arm64:
- setup CPU
- setup stack
- setup vector table
- jump to C world
so that now we can properly boot the MMU less domain.

2) Implement the proper traps handling for arm

3) Use hypercall_console_io to print messages
using printk so that we can run the hello-world test.

4) Modify the docs to reflect that we now have
one test fully supported on arm64.

Tested as a guest on qemu-system-aarch64.
Result:
(d1) - XTF booting -
(d1) - Setup CPU -
(d1) - Zero BSS -
(d1) - Setup stack -
(d1) - Jump to C world-
(d1) --- Xen Test Framework ---
(d1) Environment: ARM64 LE
(d1) Hello World
(d1) Test result: SUCCESS

Signed-off-by: Michal Orzel <[email protected]>
On MMU less system, when using XTF as dom0 we need to know
the load address as it may differ depending on the target.
Allow specifying the load address on the command line when
invoking make by using:
CONFIG_LOAD_ADDRESS=<address>

By default the load address is 0x40000000.

Signed-off-by: Michal Orzel <[email protected]>
Currently the only way to print messages from XTF
is by using Xen debug console through the hypercall
console_io. Add PL011 driver so that we can now choose
which solution we want to use.

This change adds the basic UART functionality allowing
to transmit messages in polling mode. There is no RX
support.

Early printk support is added allowing to transmit
early boot messages.

In order to compile XTF with PL011 support,
the following options must be passed when invoking make
on the command line:
CONFIG_PL011_UART=y
CONFIG_PL011_ADDRESS=<base_address>
-to compile with early printk support:
CONFIG_PL011_EARLY_PRINTK=y

Tested using XTF as dom0 on qemu-system-aarch64 and FVP Base.

Signed-off-by: Michal Orzel <[email protected]>
-setup boot page tables
-setup identity mapping
-setup fixmap for PL011 UART
-enable MMU

Configuration:
-granularity: 4KB
-VA width: 39bit
-VA start: 0xffffff8000000000
-3 levels of lookup: L1->L2->2M blocks, L2->L3->4KB pages(fixmap)
-1GB identity mapping removed before jumping into C

Add new environment called mmu64le. This way the build system
generates 2 binaries per test:
-with MMU -> test-arm-64le-<test>
-without MMU -> test-arm-mmu64le-<test>

How to build XTF on arm64 on qemu-system-aarch64:

->to use XTF as dom0:
make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CONFIG_LOAD_ADDRESS=0x80000000
-the load address will be only used for MMU-less test

->to use XTF as guest:
make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-

Signed-off-by: Michal Orzel <[email protected]>
Add macros to convert between frame numbers and address formats.
Add functions to store page table entry and setup fixmap.

Signed-off-by: Michal Orzel <[email protected]>
... together with public header features.h storing
feature flags reported by XENVER_get_features.

Signed-off-by: Michal Orzel <[email protected]>
... so that we can connect to guest console using
xl console. This change is required for OSSTEST which
makes use of xtf-runner that connects to the guest
console in order to obtain the results.

PV console should not be used when XTF is running as dom0.
For that purpose, implement a function is_initdomain
that uses hypercall xen_version to get Xen features
and checks if bit XENFEAT_dom0 is set.

Signed-off-by: Michal Orzel <[email protected]>
OSSTEST makes use of xtf-runner to run tests and obtain
the results. Add arm's environment mmu64le into xtf-runner
so that arm's tests can be executed.

Due to the arm's environment 64le being MMU-less domain
(no pv console->xtf-runner cannot parse the results),
add it into new variable called unsupported_envs to store
unsupported environments from xtf-runner point of view.
A user may want to execute such tests locally(it is good
to know how Xen behaves running MMU-less guests), hence why
we should not drop support for these environments.

Signed-off-by: Michal Orzel <[email protected]>
@dozylynx
Copy link
Contributor

Just a quick note as I'm happy to see this PR. I haven't had chance to study it yet but I can report that I've been able to build and run the provided example XTF test from the Arm64 version, building from the PR branch successfully:

root@qemuarm64:/usr/libexec/xtf# ./xtf-runner test-arm-mmu64le-example
Executing 'xl create -p tests/example/test-arm-mmu64le-example.cfg'
Executing 'xl console test-arm-mmu64le-example'
Executing 'xl unpause test-arm-mmu64le-example'
(d1) - Early printk using Xen debug console -
(d1) - XTF booting -
(d1) - Setup CPU -
(d1) - Setup page tables -
(d1) - Enable MMU -
(d1) - Zero BSS -
(d1) - Setup stack -
(d1) - Jump to C world-
(d1) --- Xen Test Framework ---
(d1) Environment: ARM64 LE MMU
(d1) Hello World
(d1) Test result: SUCCESS
--- Xen Test Framework ---
Environment: ARM64 LE MMU
Hello World
Test result: SUCCESS

Combined test results:
test-arm-mmu64le-example                 SUCCESS

Copy link
Contributor

@dozylynx dozylynx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be excellent to have merged and working - thanks for taking it on


To build XTF:
-for x86:
$ make
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ make ARCH=x86

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #5

-for arm32 natively:
$ make ARCH=arm32
-for arm32 when cross compiling:
$ make ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabi-
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(After the change is made to implement this, as discussed on Aug 17th:)
Doc should indicate that if ARCH is not specified, that it will default to target the same architecture as the build host.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #5

# Supported architectures
SUPPORTED_ARCH := x86 arm64 arm32
# Default architecture
ARCH ?= x86
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Discussed on the 17th Aug:) Switch this to default to the build host arch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #5


# Some tests are architecture specific. In this case we can have a list of tests
# supported by a given architecture in $(ROOT)/build/$(ARCH)/arch-tests.mk
-include $(ROOT)/build/$(ARCH)/arch-tests.mk
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think there could be a way to support having each test indicate its compatibility within the individual test Makefile instead? It will make it easier for maintaining downstream tests since it will avoid changes being needed in these arch-test lists.

eg. something like:
TEST-ARCHES := $(ALL_ARCHES)
like the TEST-ENVS variable.

HYPERCALL2(multicall);
HYPERCALL2(vm_assist);
HYPERCALL1(sysctl);
HYPERCALL1(domctl);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this hypercall list in any specific order?
If not, it should be sorted into an order (eg. alphabetical, or by ascending hypercall number).
A comment should be added to state what the order is, so that any future additions will be inserted into the correct place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HYPERCALL5(argo_op);
would be nice (or it can go in when a test that exercises Argo goes in).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wrappers were borrowed from Linux(with little modifications) and the list is not sorted.
It does not really matter what the order is. Preferable way is to insert additions at the end of list.
argo_op hypercall should be added together with public interface argo.h. So this should come together with a test.

int hypercall_sysctl(xen_sysctl_t *arg);
int hypercall_hvm_op(unsigned long op, void *arg);
int hypercall_grant_table_op(unsigned int cmd, void *uop, unsigned int count);
int hypercall_vcpu_op(int cmd, int vcpuid, void *extra_args);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+int hypercall_argo_op(unsigned int cmd, void *arg1, void *arg2,
+                      unsigned long arg3, unsigned long arg4);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hypercall_argo_op should be added together with argo.h public interface. This shall be added when adding a test for argo.

# arm needs linking normally, then converting to a binary format
define build-arm
$(LD) $$(LDFLAGS_$(1)) $$(DEPS-$(1)) -o $$@-syms
$(OBJCOPY) $$@-syms -O binary $$@
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(discussed on Aug 17th call) - add a TODO comment here? re: ability to use objdump and gdb with the test binary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that currentlu Xen on arm does not support booting ELF images.
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/kernel.c;h=8f43caa1866d87be9e1f0f3ca3db30ecfb9e1951;hb=HEAD#l428
It is not hard to add such support into Xen but then XTF would be the only use case for it.
We may need to ask Julien for an opinion.

Anyway the current solution is the only way to get it working. Adding TODO would indicate that other method is possible but currently it is not.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not say "it is not hard". I attempted it in 2013 (I think the patch was [1]) for FreeBSD and this is a mess to handle in libxc.

IIUC, @dozylynx is mostly interested to have objdump & gdb working. Would it be acceptable if we provide an ELF along with the Image to load? (FWIW this is what both Xen and Linux has been doing).

[1] https://lore.kernel.org/xen-devel/[email protected]/

Copy link
Contributor Author

@orzelmichal orzelmichal Aug 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are already providing ELF with raw image. The ELF is called "test"-syms and raw image is "test"
So we have the same behavior as Xen and Linux

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should've been clearer in the comment - this came up in discussion on the call from query by @andyhhp about the ability to connect gdb to a running XTF guest, if the guest is running the same binary as the one that contains the symbols.
@orzelmichal is correct that at the moment Xen doesn't have the necessary support and really the TODO should be added somewhere in Xen to add enough ELF support to enable this so I'm fine with not having a change made to this code for this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should've been clearer in the comment - this came up in discussion on the call from query by @andyhhp about the ability to connect gdb to a running XTF guest, if the guest is running the same binary as the one that contains the symbols.

I am probably missing some context here. Why do you need the symbols to be contained in the binary? IOW, what is the problem with having the symbols separated from the binary (which BTW is the way both Linux and Xen on Arm works)?.

However, AFAIK there is no support in the hypervisor to connect GDB to a guest. So you will probably want to look at that first.

@orzelmichal is correct that at the moment Xen doesn't have the necessary support and really the TODO should be added somewhere in Xen to add enough ELF support to enable this so I'm fine with not having a change made to this code for this.

Right. There is no support for good reason so far. AFAIU, In order to boot, the ELF needs to contain some Xen ELF notes. This is also a pain to add the loading in libxc/Xen for Arm (see the link I posted earlier). In fact, we dropped the support in Xen because it was broken.

It is a lot easier to tell someone to switch to the Image protocol. It has also the benefits to have a well-defined state of the machine at boot. For instance, with ELF there is no word about whether the RAM will be clean to PoC or else.

So before doing yet another attempt to add support for ELF in libxc/Xen, it would be good to understand what you can't do with Image (assuming the symbols are present) and why ELF solves it.


$ git clone git://xenbits.xen.org/xtf.git
$ cd xtf
# To build for x86:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default will be updated to build for the host architecture; to build for x86 will need to specify: ARCH=x86

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #5

* * VCPUOP_register_vcpu_info
* * VCPUOP_register_runstate_memory_area
*
*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 *  HYPERVISOR_argo_op
 *   All sub-operations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even the latest arch-arm.h public interface in Xen does not mention this hypercall.
We should not add anything into the public interface that is not in the Xen's one.

So when adding argo test, please add support for hypercall argo_op + argo.h public interface but do not modify arch-arm.h

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can one of you send a patch to xen-devel? We can then re-sync the headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will push it in a moment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

#include <xtf/numbers.h>

/* 4kB pages */
#define PAGE_SHIFT 12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is defining these PAGE_* values now going to bite later when we have a hypervisor build or guests (and tests) with non-4K page size?
ie. Would it make sense to undefine this and have an explicitly named PAGE_SIZE_4K etc. instead? There are not many references to PAGE_SIZE in this codebase yet...

* - x1(end) - end address of a region
* Clobbers: x2, x3, x4
*/
ENTRY(flush_dcache_range)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was difficult to understand whether the function would clean or just clean & invalidate the cache. How about renaming the function to clean_and_invalidate_dcache()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will rename it to clean_and_invalidate_dcache


PRINT("- Zero BSS -\n")

load_paddr x0, __start_bss
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find a bit odd that the arm32 code and arm64 patches already diverge in this commit. I think it would be better if that code stay in par in each commit touch 32-bit and 64-bit.

If you want to add more code for arm64, that's fine. But I would suggest to do it separately.

load_paddr x0, __start_bss
load_paddr x1, __end_bss

bl flush_dcache_range
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to document in the code why the flush is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will


/* Load BSS start address again as x0 has been modified in the upper loop */
load_paddr x0, __start_bss
bl flush_dcache_range
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to document in the code why the flush is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will

#define __HVC(imm16) .long \
((0xE1400070 | (((imm16) & 0xFFF0) << 4) | ((imm16) & 0x000F)) & 0xFFFFFFFF)

#define HYPERCALL_SIMPLE(hypercall) \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both .S was borrowed from Linux right? If so, that's fine because the license is not GPL. But please retain the copyright from the author.

@jgrall
Copy link

jgrall commented Aug 18, 2021

The commit message of arm64: Run hello-world test on arm64 suggests that the hello world works after that commit. But I am a bit puzzled how this can work given that you are still running with MMU off (there is no guarantee printing works) and you seem to add support for the console afterwards.

b xtf_main
ENDFUNC(_start)

ENTRY(_mmu_disable)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am not mistaken, ENTRY will export the symbol. However, this function an most of the other below are not meant to be used outside of the file. So I think you want to drop ENTRY.

Also, can you please document what each function does and more importantly mention whether they clobber any registers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will drop ENTRY for local functions and I will add comments about clobbering


/* Turn off D-cache and MMU */
mrs x2, sctlr_el1
bic x2, x2, #SCTLR_M
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Image protocol mandates that the MMU is off before jumping to the kernel. So I think you can drop this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

#define BAD_FIQ 2
#define BAD_ERROR 3

.macro invalid, reason
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the code in this commit (this is one example) looks suspiciously similar to Xen code. Did you look at Xen code while writing XTF?

Copy link
Contributor Author

@orzelmichal orzelmichal Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some parts, I looked at Xen and Linux. If there is anything wrong with it, please tell me.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some parts, I looked at Xen and Linux. If there is anything wrong with it, please tell me.

There is a licensing issue. AFAIK, it is fine to import BSD code in GPL but the invert is not. So when writing BSD code, you should avoid to read Linux/Xen code to avoid been biased.

If you look for OS example, then this should come from a BSD OS.

#define XTF_ARM_CONFIG_H

/*
* On MMU less system, when using XTF as dom0 we need to know the load address
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why you mention specifically dom0. This is also true for a domain because we don't guarantee that XTF will be loaded at the exact same place on each Xen versions.

Also, we may have spoke about it in the past but I can't remember the outcome, running XTF without MMU is going to require quite some works because Xen will work against you (we map the memory cached). So I am not sure we want to support MMU off use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that maintaining MMU off use case is hard. I'm ok to drop it as the code will be cleaner but I need others opinion.
At first I thought it'll be good to test Xen with MMU off domain because currently I do not see such scenario being tested.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I thought it'll be good to test Xen with MMU off domain because currently I do not see such scenario being tested.

They are not supported on Xen. Do you have any requirements to run workload with MMU off (other than the early boot)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd need MMU-less so that in the future we can change it to MPU domain.

Copy link

@jgrall jgrall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the early boot code is going output on the hypercall console (which is not enabled for guest on prod). But they are debugging message, so I think it would be fine.

For the runtime output, the PV console should always be there. So can you clarify why we want to add PL011 here?

* xb: register which contains the UART base address
* c: scratch register number
*/
.macro pl011_uart_init xb, c
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the name is different, surely this was heavily inspired from debug-pl011.inc in Xen.

If that's correct, I am afraid this will likely have to be fully re-written because the license are not compatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XTF is BSD-2. Xen is GPL. BSD was not compatible with GPL but BSD-2 is

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

XTF is BSD-2. Xen is GPL. BSD was not compatible with GPL but BSD-2 is

IANAL. My understanding is it means you can import BSD-2 code in GPL code but not the inverse because GPL requires you to distribute the code with the binary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got confirmation that we cannot import GPL code into BSD-2. I will keep it in mind when searching for inspiration :)

{
uint32_t val;
#ifdef CONFIG_ARM_64
asm volatile("ldr %w0, [%1]\n" : "=r" (val) : "r" (addr));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about introducing ioread and iowrite? So other driver in the future can use it without ifdefing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely it would be better. I will introduce them.

}
}

void pl011_init(void)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this looks basically a copy of the assembly version. How about just always initializing the PL011 from assembly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that would work. I'm just having the second thought whether XTF on arm should have support for PL011. After all what we really need is to have debug messages using console_io hypercall and PV console. Not sure if PL011 will be used by anyone.

@orzelmichal orzelmichal mentioned this pull request Mar 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants