* [PATCH] sbi: fwft: clear the config lock when warm boot
@ 2024-11-15 7:33 Nick Hu
2024-11-22 2:45 ` Xiang W
2024-11-28 6:44 ` Anup Patel
0 siblings, 2 replies; 5+ messages in thread
From: Nick Hu @ 2024-11-15 7:33 UTC (permalink / raw)
To: opensbi
The hotplug cpu should clear the fwft config's SBI_FWFT_SET_FLAG_LOCK
in the warm boot flow otherwise the cpu can't set the menvcfg.sse by
SBI_EXT_FWFT_SET sbi call and cause the illegal instruction when
accessing the CSR_SSP in kernel.
Signed-off-by: Nick Hu <nick.hu@sifive.com>
---
lib/sbi/sbi_fwft.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
index 9cf14c16..0cb165b4 100644
--- a/lib/sbi/sbi_fwft.c
+++ b/lib/sbi/sbi_fwft.c
@@ -311,6 +311,18 @@ static int fwft_get_feature(enum sbi_fwft_feature_t feature,
return SBI_SUCCESS;
}
+static void fwft_clear_config_lock(enum sbi_fwft_feature_t feature)
+{
+ int ret;
+ struct fwft_config *conf;
+
+ ret = fwft_get_feature(feature, &conf);
+ if (ret)
+ return;
+
+ conf->flags &= ~SBI_FWFT_SET_FLAG_LOCK;
+}
+
int sbi_fwft_set(enum sbi_fwft_feature_t feature, unsigned long value,
unsigned long flags)
{
@@ -414,5 +426,8 @@ int sbi_fwft_init(struct sbi_scratch *scratch, bool cold_boot)
fwft_set_hart_state_ptr(scratch, fhs);
}
+ for (i = 0; i < array_size(features); i++)
+ fwft_clear_config_lock(features[i].id);
+
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH] sbi: fwft: clear the config lock when warm boot
2024-11-15 7:33 [PATCH] sbi: fwft: clear the config lock when warm boot Nick Hu
@ 2024-11-22 2:45 ` Xiang W
2024-11-22 3:24 ` Samuel Holland
2024-11-28 6:44 ` Anup Patel
1 sibling, 1 reply; 5+ messages in thread
From: Xiang W @ 2024-11-22 2:45 UTC (permalink / raw)
To: opensbi
? 2024-11-15?? 15:33 +0800?Nick Hu???
> The hotplug cpu should clear the fwft config's SBI_FWFT_SET_FLAG_LOCK
> in the warm boot flow otherwise the cpu can't set the menvcfg.sse by
> SBI_EXT_FWFT_SET sbi call and cause the illegal instruction when
> accessing the CSR_SSP in kernel.
>
> Signed-off-by: Nick Hu <nick.hu@sifive.com>
In sbi_fwft_init, the memory for fwft_hart_state is allocated by?
sbi_zalloc, which ensures that flags is 0. So no additional code
is needed to clear the SBI_FWFT_SET_FLAG_LOCK bit.
Regards,
Xiang W
> ---
> ?lib/sbi/sbi_fwft.c | 15 +++++++++++++++
> ?1 file changed, 15 insertions(+)
>
> diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
> index 9cf14c16..0cb165b4 100644
> --- a/lib/sbi/sbi_fwft.c
> +++ b/lib/sbi/sbi_fwft.c
> @@ -311,6 +311,18 @@ static int fwft_get_feature(enum sbi_fwft_feature_t feature,
> ? return SBI_SUCCESS;
> ?}
> ?
> +static void fwft_clear_config_lock(enum sbi_fwft_feature_t feature)
> +{
> + int ret;
> + struct fwft_config *conf;
> +
> + ret = fwft_get_feature(feature, &conf);
> + if (ret)
> + return;
> +
> + conf->flags &= ~SBI_FWFT_SET_FLAG_LOCK;
> +}
> +
> ?int sbi_fwft_set(enum sbi_fwft_feature_t feature, unsigned long value,
> ? unsigned long flags)
> ?{
> @@ -414,5 +426,8 @@ int sbi_fwft_init(struct sbi_scratch *scratch, bool cold_boot)
> ? fwft_set_hart_state_ptr(scratch, fhs);
> ? }
> ?
> + for (i = 0; i < array_size(features); i++)
> + fwft_clear_config_lock(features[i].id);
> +
> ? return 0;
> ?}
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] sbi: fwft: clear the config lock when warm boot
2024-11-22 2:45 ` Xiang W
@ 2024-11-22 3:24 ` Samuel Holland
2024-11-22 4:06 ` Xiang W
0 siblings, 1 reply; 5+ messages in thread
From: Samuel Holland @ 2024-11-22 3:24 UTC (permalink / raw)
To: opensbi
Hi Xiang,
On 2024-11-21 8:45 PM, Xiang W wrote:
> ? 2024-11-15?? 15:33 +0800?Nick Hu???
>> The hotplug cpu should clear the fwft config's SBI_FWFT_SET_FLAG_LOCK
>> in the warm boot flow otherwise the cpu can't set the menvcfg.sse by
>> SBI_EXT_FWFT_SET sbi call and cause the illegal instruction when
>> accessing the CSR_SSP in kernel.
>>
>> Signed-off-by: Nick Hu <nick.hu@sifive.com>
>
> In sbi_fwft_init, the memory for fwft_hart_state is allocated by?
> sbi_zalloc, which ensures that flags is 0. So no additional code
> is needed to clear the SBI_FWFT_SET_FLAG_LOCK bit.
That allocation only occurs the first time a hart is started. After a
sbi_hsm_hart_stop() and sbi_hsm_hart_start(), the FWFT features will be reset to
their default values per the SBI spec, and the flags need to be reset as well.
Regards,
Samuel
>> ---
>> ?lib/sbi/sbi_fwft.c | 15 +++++++++++++++
>> ?1 file changed, 15 insertions(+)
>>
>> diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
>> index 9cf14c16..0cb165b4 100644
>> --- a/lib/sbi/sbi_fwft.c
>> +++ b/lib/sbi/sbi_fwft.c
>> @@ -311,6 +311,18 @@ static int fwft_get_feature(enum sbi_fwft_feature_t feature,
>> ? return SBI_SUCCESS;
>> ?}
>> ?
>> +static void fwft_clear_config_lock(enum sbi_fwft_feature_t feature)
>> +{
>> + int ret;
>> + struct fwft_config *conf;
>> +
>> + ret = fwft_get_feature(feature, &conf);
>> + if (ret)
>> + return;
>> +
>> + conf->flags &= ~SBI_FWFT_SET_FLAG_LOCK;
>> +}
>> +
>> ?int sbi_fwft_set(enum sbi_fwft_feature_t feature, unsigned long value,
>> ? unsigned long flags)
>> ?{
>> @@ -414,5 +426,8 @@ int sbi_fwft_init(struct sbi_scratch *scratch, bool cold_boot)
>> ? fwft_set_hart_state_ptr(scratch, fhs);
>> ? }
>> ?
>> + for (i = 0; i < array_size(features); i++)
>> + fwft_clear_config_lock(features[i].id);
>> +
>> ? return 0;
>> ?}
>> --
>> 2.34.1
>>
>>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] sbi: fwft: clear the config lock when warm boot
2024-11-22 3:24 ` Samuel Holland
@ 2024-11-22 4:06 ` Xiang W
0 siblings, 0 replies; 5+ messages in thread
From: Xiang W @ 2024-11-22 4:06 UTC (permalink / raw)
To: opensbi
? 2024-11-21?? 21:24 -0600?Samuel Holland???
> Hi Xiang,
>
> On 2024-11-21 8:45 PM, Xiang W wrote:
> > ? 2024-11-15?? 15:33 +0800?Nick Hu???
> > > The hotplug cpu should clear the fwft config's SBI_FWFT_SET_FLAG_LOCK
> > > in the warm boot flow otherwise the cpu can't set the menvcfg.sse by
> > > SBI_EXT_FWFT_SET sbi call and cause the illegal instruction when
> > > accessing the CSR_SSP in kernel.
> > >
> > > Signed-off-by: Nick Hu <nick.hu@sifive.com>
> >
> > In sbi_fwft_init, the memory for fwft_hart_state is allocated by?
> > sbi_zalloc, which ensures that flags is 0. So no additional code
> > is needed to clear the SBI_FWFT_SET_FLAG_LOCK bit.
>
> That allocation only occurs the first time a hart is started. After a
> sbi_hsm_hart_stop() and sbi_hsm_hart_start(), the FWFT features will be reset to
> their default values per the SBI spec, and the flags need to be reset as well.
>
Yes, you are right, I ignored the situation you mentioned.
Regards,
Xiang W
> Regards,
> Samuel
>
> > > ---
> > > ?lib/sbi/sbi_fwft.c | 15 +++++++++++++++
> > > ?1 file changed, 15 insertions(+)
> > >
> > > diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
> > > index 9cf14c16..0cb165b4 100644
> > > --- a/lib/sbi/sbi_fwft.c
> > > +++ b/lib/sbi/sbi_fwft.c
> > > @@ -311,6 +311,18 @@ static int fwft_get_feature(enum sbi_fwft_feature_t feature,
> > > ? return SBI_SUCCESS;
> > > ?}
> > > ?
> > > +static void fwft_clear_config_lock(enum sbi_fwft_feature_t feature)
> > > +{
> > > + int ret;
> > > + struct fwft_config *conf;
> > > +
> > > + ret = fwft_get_feature(feature, &conf);
> > > + if (ret)
> > > + return;
> > > +
> > > + conf->flags &= ~SBI_FWFT_SET_FLAG_LOCK;
> > > +}
> > > +
> > > ?int sbi_fwft_set(enum sbi_fwft_feature_t feature, unsigned long value,
> > > ? unsigned long flags)
> > > ?{
> > > @@ -414,5 +426,8 @@ int sbi_fwft_init(struct sbi_scratch *scratch, bool cold_boot)
> > > ? fwft_set_hart_state_ptr(scratch, fhs);
> > > ? }
> > > ?
> > > + for (i = 0; i < array_size(features); i++)
> > > + fwft_clear_config_lock(features[i].id);
> > > +
> > > ? return 0;
> > > ?}
> > > --
> > > 2.34.1
> > >
> > >
> >
> >
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] sbi: fwft: clear the config lock when warm boot
2024-11-15 7:33 [PATCH] sbi: fwft: clear the config lock when warm boot Nick Hu
2024-11-22 2:45 ` Xiang W
@ 2024-11-28 6:44 ` Anup Patel
1 sibling, 0 replies; 5+ messages in thread
From: Anup Patel @ 2024-11-28 6:44 UTC (permalink / raw)
To: opensbi
On Fri, Nov 15, 2024 at 1:03?PM Nick Hu <nick.hu@sifive.com> wrote:
>
> The hotplug cpu should clear the fwft config's SBI_FWFT_SET_FLAG_LOCK
> in the warm boot flow otherwise the cpu can't set the menvcfg.sse by
> SBI_EXT_FWFT_SET sbi call and cause the illegal instruction when
> accessing the CSR_SSP in kernel.
>
> Signed-off-by: Nick Hu <nick.hu@sifive.com>
LGTM.
Reviewed-by: Anup Patel <anup@brainfault.org>
Applied this patch to the riscv/opensbi repo.
Thanks,
Anup
> ---
> lib/sbi/sbi_fwft.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
> index 9cf14c16..0cb165b4 100644
> --- a/lib/sbi/sbi_fwft.c
> +++ b/lib/sbi/sbi_fwft.c
> @@ -311,6 +311,18 @@ static int fwft_get_feature(enum sbi_fwft_feature_t feature,
> return SBI_SUCCESS;
> }
>
> +static void fwft_clear_config_lock(enum sbi_fwft_feature_t feature)
> +{
> + int ret;
> + struct fwft_config *conf;
> +
> + ret = fwft_get_feature(feature, &conf);
> + if (ret)
> + return;
> +
> + conf->flags &= ~SBI_FWFT_SET_FLAG_LOCK;
> +}
> +
> int sbi_fwft_set(enum sbi_fwft_feature_t feature, unsigned long value,
> unsigned long flags)
> {
> @@ -414,5 +426,8 @@ int sbi_fwft_init(struct sbi_scratch *scratch, bool cold_boot)
> fwft_set_hart_state_ptr(scratch, fhs);
> }
>
> + for (i = 0; i < array_size(features); i++)
> + fwft_clear_config_lock(features[i].id);
> +
> return 0;
> }
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-11-28 6:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-15 7:33 [PATCH] sbi: fwft: clear the config lock when warm boot Nick Hu
2024-11-22 2:45 ` Xiang W
2024-11-22 3:24 ` Samuel Holland
2024-11-22 4:06 ` Xiang W
2024-11-28 6:44 ` Anup Patel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).