| Message ID | 20200227204023.22125-14-grimm@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Ultravisor support in skiboot | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (82aed17a5468aff6b600ee1694a10a60f942c018) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On 28/02/2020 07:40, Ryan Grimm wrote: > From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > > Remove the IMC nodes when the ultravisor is enabled, > since both HOMER and IMC scoms are not accessable in > hypervisor state. They are available via UV_READ_SCOM. > > Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > --- > hw/imc.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/imc.c b/hw/imc.c > index 3a5382c0..576eac87 100644 > --- a/hw/imc.c > +++ b/hw/imc.c > @@ -603,6 +603,17 @@ imc_mambo: > if (pause_microcode_at_boot()) > goto err; > > + /* > + * If MSR(S) bit is set, disable IMC nodes. > + * IMC nodes need access to specific scom and HOMER region > + * which are not accessible from hypervisor. > + * > + * At this point uv_present cant be used since uv_init() s/cant/can't/ s/uv_init/init_uv/ > + * is called much later. Hencing checking for the MSR bit here. > + */ > + if (is_msr_bit_set(MSR_S)) > + goto err; > + Do this at the very beginning of imc_init()? Or do not call it at all in main_cpu_entry() if (is_msr_bit_set(MSR_S))? > /* > * If the dt_attach_root() fails, "imc-counters" node will not be > * seen in the device-tree and hence OS should not make any >
On 3/12/20 6:49 AM, Alexey Kardashevskiy wrote: > > On 28/02/2020 07:40, Ryan Grimm wrote: >> From: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> >> >> Remove the IMC nodes when the ultravisor is enabled, >> since both HOMER and IMC scoms are not accessable in >> hypervisor state. > > They are available via UV_READ_SCOM. SCOM yes, but the latency will be high and also there are security concerns, hence to disable it. > > >> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> >> --- >> hw/imc.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/hw/imc.c b/hw/imc.c >> index 3a5382c0..576eac87 100644 >> --- a/hw/imc.c >> +++ b/hw/imc.c >> @@ -603,6 +603,17 @@ imc_mambo: >> if (pause_microcode_at_boot()) >> goto err; >> >> + /* >> + * If MSR(S) bit is set, disable IMC nodes. >> + * IMC nodes need access to specific scom and HOMER region >> + * which are not accessible from hypervisor. >> + * >> + * At this point uv_present cant be used since uv_init() > s/cant/can't/ > s/uv_init/init_uv/ Will fix it. > > >> + * is called much later. Hencing checking for the MSR bit here. >> + */ >> + if (is_msr_bit_set(MSR_S)) >> + goto err; >> + > Do this at the very beginning of imc_init()? Or do not call it at all in > main_cpu_entry() if (is_msr_bit_set(MSR_S))? We do couple of call to this file before jumping to imc_init(). And yes I agree, we could move the check to the beginning. Will fix it Thanks for review maddy > > >> /* >> * If the dt_attach_root() fails, "imc-counters" node will not be >> * seen in the device-tree and hence OS should not make any >>
diff --git a/hw/imc.c b/hw/imc.c index 3a5382c0..576eac87 100644 --- a/hw/imc.c +++ b/hw/imc.c @@ -603,6 +603,17 @@ imc_mambo: if (pause_microcode_at_boot()) goto err; + /* + * If MSR(S) bit is set, disable IMC nodes. + * IMC nodes need access to specific scom and HOMER region + * which are not accessible from hypervisor. + * + * At this point uv_present cant be used since uv_init() + * is called much later. Hencing checking for the MSR bit here. + */ + if (is_msr_bit_set(MSR_S)) + goto err; + /* * If the dt_attach_root() fails, "imc-counters" node will not be * seen in the device-tree and hence OS should not make any