| Message ID | 20230919112827.1001484-2-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: ITS: implement TODO and fix issues with cache | expand |
Hi Volodymyr, On 19/09/2023 12:28, Volodymyr Babchuk wrote: > Implement TODO by calling the INVALL command. I think the TODO was meant to be an optimization because AFAICT we are already sending an INV command per event. > It is working on real > HW, so there is no sense in not doing this. A patch should be justified based from the spec or an errata. Not that fact it works on a real HW. At the moment, I don't quite understand why you need one because as said above, we are technically already sending an INV per event so we should be covered. Removing the INV and using INVALL would make sense as an optimization. Yet given the current code doesn't seem to work, I would like to understand what's the problem of using INV. > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> > --- > xen/arch/arm/gic-v3-its.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c > index 3aa4edda10..a9c971a55f 100644 > --- a/xen/arch/arm/gic-v3-its.c > +++ b/xen/arch/arm/gic-v3-its.c > @@ -236,6 +236,19 @@ static int its_send_cmd_inv(struct host_its *its, > return its_send_command(its, cmd); > } > > +static int its_send_cmd_invall(struct host_its *its, > + uint32_t collection_id) > +{ > + uint64_t cmd[4]; > + > + cmd[0] = GITS_CMD_INVALL; > + cmd[1] = 0x00; > + cmd[2] = collection_id; > + cmd[3] = 0x00; > + > + return its_send_command(its, cmd); > +} > + > /* Set up the (1:1) collection mapping for the given host CPU. */ > int gicv3_its_setup_collection(unsigned int cpu) > { > @@ -593,7 +606,9 @@ static int gicv3_its_map_host_events(struct host_its *its, > return ret; > } > > - /* TODO: Consider using INVALL here. Didn't work on the model, though. */ > + ret = its_send_cmd_invall(its, 0); > + if ( ret ) > + return ret; > > ret = its_send_cmd_sync(its, 0); > if ( ret ) Cheers,
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 3aa4edda10..a9c971a55f 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -236,6 +236,19 @@ static int its_send_cmd_inv(struct host_its *its, return its_send_command(its, cmd); } +static int its_send_cmd_invall(struct host_its *its, + uint32_t collection_id) +{ + uint64_t cmd[4]; + + cmd[0] = GITS_CMD_INVALL; + cmd[1] = 0x00; + cmd[2] = collection_id; + cmd[3] = 0x00; + + return its_send_command(its, cmd); +} + /* Set up the (1:1) collection mapping for the given host CPU. */ int gicv3_its_setup_collection(unsigned int cpu) { @@ -593,7 +606,9 @@ static int gicv3_its_map_host_events(struct host_its *its, return ret; } - /* TODO: Consider using INVALL here. Didn't work on the model, though. */ + ret = its_send_cmd_invall(its, 0); + if ( ret ) + return ret; ret = its_send_cmd_sync(its, 0); if ( ret )
Implement TODO by calling the INVALL command. It is working on real HW, so there is no sense in not doing this. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- xen/arch/arm/gic-v3-its.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)