| Message ID | 20200122151354.23683-5-grimm@linux.ibm.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | Ultravisor support in skiboot | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch master (d75e82dbfbb9443efeb3f9a5921ac23605aab469) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
On Wed, 2020-01-22 at 10:13 -0500, Ryan Grimm wrote: > From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > > xscom registers are in the secure memory area when secure mode is > enabled. These registers cannot be accessed directly and need to use > ultravisor services using ultracall. > > Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > Signed-off-by: Santosh Sivaraj <santosh@fossix.org> > [ linuxram: Set uv_present just after starting UV ] > Signed-off-by: Ram Pai <linuxram@us.ibm.com> > [ grimm: Don't check MSR in xscom read/write ] > Signed-off-by: Ryan Grimm <grimm@linux.ibm.com> > --- > hw/ultravisor.c | 4 ++++ > include/ultravisor.h | 22 ++++++++++++++++++++++ > include/xscom.h | 11 +++++++++-- > 3 files changed, 35 insertions(+), 2 deletions(-) > > diff --git a/hw/ultravisor.c b/hw/ultravisor.c > index d55d752b..a5553757 100644 > --- a/hw/ultravisor.c > +++ b/hw/ultravisor.c > @@ -17,6 +17,7 @@ > #include <libfdt/libfdt.h> > #include <libstb/container.h> > > +bool uv_present = false; > static char *uv_image = NULL; > static size_t uv_image_size; > struct xz_decompress *uv_xz = NULL; > @@ -251,6 +252,9 @@ int start_ultravisor(void) > > cpu_start_ultravisor((void *)uv_opal); > > + /* From now on XSCOM must go through ucall */ > + uv_present = true; > + > while (i > 0) > cpu_wait_job(jobs[--i], true); > > diff --git a/include/ultravisor.h b/include/ultravisor.h > index daeb99b6..d212f463 100644 > --- a/include/ultravisor.h > +++ b/include/ultravisor.h > @@ -12,10 +12,15 @@ > * for the secure virtual machines */ > #define UV_SECURE_MEM_BIT (PPC_BIT(15)) > #define MAX_COMPRESSED_UV_IMAGE_SIZE 0x40000 /* 256 Kilobytes */ > +#define UV_READ_SCOM 0xF114 > +#define UV_WRITE_SCOM 0xF118 > +#define UCALL_BUFSIZE 4 From 1/6: > Return value of all ultracalls is in register R3. Other output values > from the ultracall, if any, are returned in registers R4 through R12. Which is up to 9 returned values. So where did 4 come from? This is an accident waiting to happen. > #define UV_ACCESS_BIT 0x1ULL << 48 > #define UV_LOAD_MAX_SIZE 0x200000 > #define UV_FDT_MAX_SIZE 0x100000 > > +extern bool uv_present; > + > extern int start_uv(uint64_t entry, struct uv_opal *uv_opal); > extern bool uv_add_mem_range(__be64 start, __be64 end); > extern void uv_preload_image(void); > @@ -24,4 +29,21 @@ extern void init_uv(void); > extern int start_ultravisor(void); > extern long ucall(unsigned long opcode, unsigned long *retbuf, ...); > > +static inline int uv_xscom_read(u64 partid, u64 pcb_addr, u64 *val) > +{ > + long rc; > + unsigned long retbuf[UCALL_BUFSIZE]; > + > + rc = ucall(UV_READ_SCOM, retbuf, partid, pcb_addr); > + *val = retbuf[0]; > + return rc; > +} > + > +static inline int uv_xscom_write(u64 partid, u64 pcb_addr, u64 val) > +{ > + unsigned long retbuf[UCALL_BUFSIZE]; > + > + return ucall(UV_WRITE_SCOM, retbuf, partid, pcb_addr, val); > +} Do we need to be logging xscom errors in skiboot? > + > #endif /* __ULTRAVISOR_H */ > diff --git a/include/xscom.h b/include/xscom.h > index 8a466d56..2f4c819b 100644 > --- a/include/xscom.h > +++ b/include/xscom.h > @@ -7,6 +7,7 @@ > #include <stdint.h> > #include <processor.h> > #include <cpu.h> > +#include <ultravisor.h> > > /* > * SCOM "partID" definitions: > @@ -174,10 +175,16 @@ extern void _xscom_unlock(void); > /* Targeted SCOM access */ > static inline int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val) > { > - return _xscom_read(partid, pcb_addr, val, true); > + if (!uv_present) > + return _xscom_read(partid, pcb_addr, val, true); > + > + return uv_xscom_read(partid, pcb_addr, val); > } > static inline int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val) { > - return _xscom_write(partid, pcb_addr, val, true); > + if (!uv_present) > + return _xscom_write(partid, pcb_addr, val, true); > + > + return uv_xscom_write(partid, pcb_addr, val); Seems kind of backwards since having a UV is the exceptional case. IMO do: if (uv_present) return do_uv_scom() return _scom_write(); Less noise in the diff too. > } > extern int xscom_write_mask(uint32_t partid, uint64_t pcb_addr, uint64_t val, uint64_t mask); >
On Tue, 2020-01-28 at 17:27 +1100, Oliver O'Halloran wrote: > +#define UCALL_BUFSIZE 4 > > From 1/6: > > > Return value of all ultracalls is in register R3. Other output > > values > > from the ultracall, if any, are returned in registers R4 through > > R12. > > Which is up to 9 returned values. So where did 4 come from? This is > an > accident waiting to happen. I found out this was coded up quickly to support the xscom ucall. We will follow hcall convention and post an implementation of ucall4 and ucall9. > +static inline int uv_xscom_write(u64 partid, u64 pcb_addr, u64 > > val) > > +{ > > + unsigned long retbuf[UCALL_BUFSIZE]; > > + > > + return ucall(UV_WRITE_SCOM, retbuf, partid, pcb_addr, val); > > +} > > Do we need to be logging xscom errors in skiboot? > Do you mean check RC and print to skiboot log? Or do something more? > > static inline int xscom_read(uint32_t partid, uint64_t pcb_addr, > > uint64_t *val) > > { > > - return _xscom_read(partid, pcb_addr, val, true); > > + if (!uv_present) > > + return _xscom_read(partid, pcb_addr, val, true); > > + > > + return uv_xscom_read(partid, pcb_addr, val); > > } > > static inline int xscom_write(uint32_t partid, uint64_t pcb_addr, > > uint64_t val) { > > - return _xscom_write(partid, pcb_addr, val, true); > > + if (!uv_present) > > + return _xscom_write(partid, pcb_addr, val, true); > > + > > + return uv_xscom_write(partid, pcb_addr, val); > > Seems kind of backwards since having a UV is the exceptional case. > IMO > do: > > if (uv_present) > return do_uv_scom() > return _scom_write(); > > Less noise in the diff too. > k Thanks, Ryan
diff --git a/hw/ultravisor.c b/hw/ultravisor.c index d55d752b..a5553757 100644 --- a/hw/ultravisor.c +++ b/hw/ultravisor.c @@ -17,6 +17,7 @@ #include <libfdt/libfdt.h> #include <libstb/container.h> +bool uv_present = false; static char *uv_image = NULL; static size_t uv_image_size; struct xz_decompress *uv_xz = NULL; @@ -251,6 +252,9 @@ int start_ultravisor(void) cpu_start_ultravisor((void *)uv_opal); + /* From now on XSCOM must go through ucall */ + uv_present = true; + while (i > 0) cpu_wait_job(jobs[--i], true); diff --git a/include/ultravisor.h b/include/ultravisor.h index daeb99b6..d212f463 100644 --- a/include/ultravisor.h +++ b/include/ultravisor.h @@ -12,10 +12,15 @@ * for the secure virtual machines */ #define UV_SECURE_MEM_BIT (PPC_BIT(15)) #define MAX_COMPRESSED_UV_IMAGE_SIZE 0x40000 /* 256 Kilobytes */ +#define UV_READ_SCOM 0xF114 +#define UV_WRITE_SCOM 0xF118 +#define UCALL_BUFSIZE 4 #define UV_ACCESS_BIT 0x1ULL << 48 #define UV_LOAD_MAX_SIZE 0x200000 #define UV_FDT_MAX_SIZE 0x100000 +extern bool uv_present; + extern int start_uv(uint64_t entry, struct uv_opal *uv_opal); extern bool uv_add_mem_range(__be64 start, __be64 end); extern void uv_preload_image(void); @@ -24,4 +29,21 @@ extern void init_uv(void); extern int start_ultravisor(void); extern long ucall(unsigned long opcode, unsigned long *retbuf, ...); +static inline int uv_xscom_read(u64 partid, u64 pcb_addr, u64 *val) +{ + long rc; + unsigned long retbuf[UCALL_BUFSIZE]; + + rc = ucall(UV_READ_SCOM, retbuf, partid, pcb_addr); + *val = retbuf[0]; + return rc; +} + +static inline int uv_xscom_write(u64 partid, u64 pcb_addr, u64 val) +{ + unsigned long retbuf[UCALL_BUFSIZE]; + + return ucall(UV_WRITE_SCOM, retbuf, partid, pcb_addr, val); +} + #endif /* __ULTRAVISOR_H */ diff --git a/include/xscom.h b/include/xscom.h index 8a466d56..2f4c819b 100644 --- a/include/xscom.h +++ b/include/xscom.h @@ -7,6 +7,7 @@ #include <stdint.h> #include <processor.h> #include <cpu.h> +#include <ultravisor.h> /* * SCOM "partID" definitions: @@ -174,10 +175,16 @@ extern void _xscom_unlock(void); /* Targeted SCOM access */ static inline int xscom_read(uint32_t partid, uint64_t pcb_addr, uint64_t *val) { - return _xscom_read(partid, pcb_addr, val, true); + if (!uv_present) + return _xscom_read(partid, pcb_addr, val, true); + + return uv_xscom_read(partid, pcb_addr, val); } static inline int xscom_write(uint32_t partid, uint64_t pcb_addr, uint64_t val) { - return _xscom_write(partid, pcb_addr, val, true); + if (!uv_present) + return _xscom_write(partid, pcb_addr, val, true); + + return uv_xscom_write(partid, pcb_addr, val); } extern int xscom_write_mask(uint32_t partid, uint64_t pcb_addr, uint64_t val, uint64_t mask);