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

Re: [Minios-devel] [UNIKRAFT PATCHv4 17/43] plat/include: Define macros for Arm64 to access registers





On 10/07/18 07:56, Wei Chen wrote:
Hi Julien,

Hi Wei,

-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 2018年7月9日 18:29
To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
simon.kuenzer@xxxxxxxxx
Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 17/43] plat/include: Define
macros for Arm64 to access registers



On 09/07/18 10:37, Wei Chen wrote:
Hi Julien,

Hi Wei,

-----Original Message-----
From: Julien Grall <julien.grall@xxxxxxx>
Sent: 2018年7月8日 6:13
To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
simon.kuenzer@xxxxxxxxx
Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 17/43] plat/include: Define
macros for Arm64 to access registers

Hi,

On 07/06/2018 10:03 AM, Wei Chen wrote:
In the progress of Arm64 system initialization, we need to access
the system registers to configure some CPU features, we also need
to access device registers to make device work. So in this patch,
we define macros to access device registers and system registers.

Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
---
    plat/common/include/arm/arm64/cpu.h | 72 +++++++++++++++++++++++++++++
    plat/common/include/arm/cpu.h       | 46 ++++++++++++++++++
    plat/common/include/cpu.h           |  4 +-
    3 files changed, 121 insertions(+), 1 deletion(-)
    create mode 100644 plat/common/include/arm/arm64/cpu.h
    create mode 100644 plat/common/include/arm/cpu.h

diff --git a/plat/common/include/arm/arm64/cpu.h
b/plat/common/include/arm/arm64/cpu.h
new file mode 100644
index 0000000..7c79462
--- /dev/null
+++ b/plat/common/include/arm/arm64/cpu.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Wei Chen <wei.chen@xxxxxxx>
+ *
+ * Copyright (c) 2018, Arm Ltd. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived
from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
"AS
IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS
BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#include <inttypes.h>
+
+#define REG_READ8(addr)            \
The naming is slightly confusing, you don't really now which register
you are reading. How about renaming them to ioread. This would make
clearer that you do io access.

That's fine with me. About this name, I have been commented in
different projects with different contents : )
Some guys said, you don't have IO instructions, why you use io as
Prefix ; )

IO are not only accessed via instructions (e.g outb/inb), they can also
be memory mapped as it is on Arm.

With the wording "reg", it is not clear whether you are accessing the
system register or mmio register.

But then you implement outb/inb below that does not make sense for Arm ;).


Yes, that's right : ) I implement outb/inb here just to make some common
code happy, because they are using x86's inb/outb API by default. I don't
want to make #if defined (__aarch64__) everywhere.

While I agree we don't want __aarch64__ everywhere, I am not entirely convince this is right to define them on Arm as a wrapper to ioread.

The way IO port are handled on Arm is by accessing at a specific MMIO base address. Do you have any user of this today?


I will discuss with Simon about this. Maybe we can design some interfaces
for IO, just like uk_ioread/uk_iowrite.

There are 2 way to do IO on x86:
        - IO ports accessible via in/out instruction
        - MMIO directly accessible from the memory

So I am not entirely sure you want to rename outb/inb to uk_ioread/uk_iowrite here.

Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

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