Hi,
On 25/04/2024 08:06, Henry Wang wrote:
Hi Julien,
On 4/24/2024 8:58 PM, Julien Grall wrote:
Hi Henry,
On 24/04/2024 04:34, Henry Wang wrote:
From: Vikram Garhwal <fnu.vik...@xilinx.com>
Enable interrupt assign/remove for running VMs in CONFIG_OVERLAY_DTB.
Currently, irq_route and mapping is only allowed at the domain
creation. Adding
exception for CONFIG_OVERLAY_DTB.
AFAICT, this is mostly reverting b8577547236f ("xen/arm: Restrict when
a physical IRQ can be routed/removed from/to a domain").
Signed-off-by: Vikram Garhwal <fnu.vik...@xilinx.com>
Signed-off-by: Stefano Stabellini <stefano.stabell...@xilinx.com>
Signed-off-by: Henry Wang <xin.wa...@amd.com>
---
xen/arch/arm/gic.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 44c40e86de..a775f886ed 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -140,8 +140,10 @@ int gic_route_irq_to_guest(struct domain *d,
unsigned int virq,
* back to the physical IRQ. To prevent get unsync, restrict the
* routing to when the Domain is been created.
*/
The above comment explains why the check was added. But the commit
message doesn't explain why this can be disregarded for your use-case.
Looking at the history, I don't think you can simply remove the checks.
Regardless that...
+#ifndef CONFIG_OVERLAY_DTB
... I am against such #ifdef. A distros may want to have OVERLAY_DTB
enabled, yet the user will not use it.
Instead, you want to remove the check once the code can properly
handle routing an IRQ the domain is created or ...
if ( d->creation_finished )
return -EBUSY;
+#endif
ret = vgic_connect_hw_irq(d, NULL, virq, desc, true);
if ( ret )
@@ -171,8 +173,10 @@ int gic_remove_irq_from_guest(struct domain *d,
unsigned int virq,
* Removing an interrupt while the domain is running may have
* undesirable effect on the vGIC emulation.
*/
+#ifndef CONFIG_OVERLAY_DTB
if ( !d->is_dying )
return -EBUSY;
+#endif
... removed before they domain is destroyed.
Thanks for your feeedback. After checking the b8577547236f commit
message I think I now understand your point. Do you have any suggestion
about how can I properly add the support to route/remove the IRQ to
running domains? Thanks.
I haven't really look at that code in quite a while. I think we need to
make sure that the virtual and physical IRQ state matches at the time we
do the routing.
I am undecided on whether we want to simply prevent the action to happen
or try to reset the state.
There is also the question of what to do if the guest is enabling the
vIRQ before it is routed.
Overall, someone needs to spend some time reading the code and then make
a proposal (this could be just documentation if we believe it is safe to
do). Both the current vGIC and the new one may need an update.
Cheers,
--
Julien Grall
