diff for duplicates of <1367895892.3398.13@snotra> diff --git a/a/1.txt b/N1/1.txt index d64fd06..fc3c8d7 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -4,71 +4,71 @@ On 05/06/2013 09:43:37 PM, tiejun.chen wrote: >>> On 05/07/2013 07:50 AM, Scott Wood wrote: >>>> On 05/05/2013 10:13:17 PM, tiejun.chen wrote: >>>>> On 05/06/2013 11:10 AM, Tiejun Chen wrote: ->>>>>> For the external interrupt, the decrementer exception and the =20 +>>>>>> For the external interrupt, the decrementer exception and the >>>>>> doorbell ->>>>>> excpetion, we also need to soft-disable interrupts while doing =20 +>>>>>> excpetion, we also need to soft-disable interrupts while doing >>>>>> as host ->>>>>> interrupt handlers since the DO_KVM hook is always performed to =20 +>>>>>> interrupt handlers since the DO_KVM hook is always performed to >>>>>> skip >>>>>> EXCEPTION_COMMON then miss this original chance with the 'ints' >>>>>> (INTS_DISABLE). ->>>>=20 +>>>> >>>> http://patchwork.ozlabs.org/patch/241344/ >>>> http://patchwork.ozlabs.org/patch/241412/ ->>>>=20 +>>>> >>>> :-) ->>>=20 +>>> >>> I'm observing the same behaviour as well: ->>>=20 +>>> >>> WARN_ON_ONCE(!irqs_disabled()); ->>=20 ->> So, could you explain the benefits of your approach over what's =20 +>> +>> So, could you explain the benefits of your approach over what's >> being discussed >> in those threads? ->=20 +> > They're a long thread so I think I need to take time to see :) ->=20 ->>=20 ->>>> Why wouldn't we always disable them? kvmppc_handle_exit() will =20 +> +>> +>>>> Why wouldn't we always disable them? kvmppc_handle_exit() will >>>> enable >>>> interrupts when it's ready. ->>>=20 ->>> This only disable soft interrupt for kvmppc_restart_interrupt() =20 +>>> +>>> This only disable soft interrupt for kvmppc_restart_interrupt() >>> that restarts >>> interrupts if they were meant for the host: ->>>=20 +>>> >>> a. SOFT_DISABLE_INTS() only for BOOKE_INTERRUPT_EXTERNAL | >>> BOOKE_INTERRUPT_DECREMENTER | BOOKE_INTERRUPT_DOORBELL ->>=20 ->> Those aren't the only exceptions that can end up going to the host. =20 +>> +>> Those aren't the only exceptions that can end up going to the host. >> We could >> get a TLB miss that results in a heavyweight MMIO exit, etc. ->=20 -> This is like host handler, so I'm just disabling soft interrupt =20 -> during kvmppc_restart_interrupt() for Doorbell interrupt/Decrementer =20 +> +> This is like host handler, so I'm just disabling soft interrupt +> during kvmppc_restart_interrupt() for Doorbell interrupt/Decrementer > Interrupt/External Input Interrupt. ->=20 -> I don't see anything should be disabled for any TLB exception in host =20 +> +> I don't see anything should be disabled for any TLB exception in host > handler. -Every exception type needs consistent lazy EE state once we hit the =20 +Every exception type needs consistent lazy EE state once we hit the local_irq_enable() (or any other C code that cares). -Plus, if you're going to add code to make something conditional, you =20 -should have a good reason for making it conditional. Being more like =20 -the 64-bit host handler for its own sake isn't good enough, especially =20 +Plus, if you're going to add code to make something conditional, you +should have a good reason for making it conditional. Being more like +the 64-bit host handler for its own sake isn't good enough, especially if you introduce differences between 32 and 64 bit in the process. >> And I'd rather see any fix for this problem stay out of the asm code. ->=20 -> We already have an appropriate SOFT_DISABLE_INTS so I think we can =20 +> +> We already have an appropriate SOFT_DISABLE_INTS so I think we can > take this easily :) -An unconditional call to hard_irq_disable() at the beginning of =20 -kvmppc_handle_exit() should suffice. I already have this in the next =20 +An unconditional call to hard_irq_disable() at the beginning of +kvmppc_handle_exit() should suffice. I already have this in the next version of my patch that I'll be posting shortly. -Note that we need this on 32-bit as well, so that trace_hardirqs_off() =20 +Note that we need this on 32-bit as well, so that trace_hardirqs_off() gets called. --Scott= +-Scott diff --git a/a/content_digest b/N1/content_digest index 587b9f2..69f89eb 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -15,73 +15,73 @@ ">>> On 05/07/2013 07:50 AM, Scott Wood wrote:\n" ">>>> On 05/05/2013 10:13:17 PM, tiejun.chen wrote:\n" ">>>>> On 05/06/2013 11:10 AM, Tiejun Chen wrote:\n" - ">>>>>> For the external interrupt, the decrementer exception and the =20\n" + ">>>>>> For the external interrupt, the decrementer exception and the \n" ">>>>>> doorbell\n" - ">>>>>> excpetion, we also need to soft-disable interrupts while doing =20\n" + ">>>>>> excpetion, we also need to soft-disable interrupts while doing \n" ">>>>>> as host\n" - ">>>>>> interrupt handlers since the DO_KVM hook is always performed to =20\n" + ">>>>>> interrupt handlers since the DO_KVM hook is always performed to \n" ">>>>>> skip\n" ">>>>>> EXCEPTION_COMMON then miss this original chance with the 'ints'\n" ">>>>>> (INTS_DISABLE).\n" - ">>>>=20\n" + ">>>> \n" ">>>> http://patchwork.ozlabs.org/patch/241344/\n" ">>>> http://patchwork.ozlabs.org/patch/241412/\n" - ">>>>=20\n" + ">>>> \n" ">>>> :-)\n" - ">>>=20\n" + ">>> \n" ">>> I'm observing the same behaviour as well:\n" - ">>>=20\n" + ">>> \n" ">>> WARN_ON_ONCE(!irqs_disabled());\n" - ">>=20\n" - ">> So, could you explain the benefits of your approach over what's =20\n" + ">> \n" + ">> So, could you explain the benefits of your approach over what's \n" ">> being discussed\n" ">> in those threads?\n" - ">=20\n" + "> \n" "> They're a long thread so I think I need to take time to see :)\n" - ">=20\n" - ">>=20\n" - ">>>> Why wouldn't we always disable them? kvmppc_handle_exit() will =20\n" + "> \n" + ">> \n" + ">>>> Why wouldn't we always disable them? kvmppc_handle_exit() will \n" ">>>> enable\n" ">>>> interrupts when it's ready.\n" - ">>>=20\n" - ">>> This only disable soft interrupt for kvmppc_restart_interrupt() =20\n" + ">>> \n" + ">>> This only disable soft interrupt for kvmppc_restart_interrupt() \n" ">>> that restarts\n" ">>> interrupts if they were meant for the host:\n" - ">>>=20\n" + ">>> \n" ">>> a. SOFT_DISABLE_INTS() only for BOOKE_INTERRUPT_EXTERNAL |\n" ">>> BOOKE_INTERRUPT_DECREMENTER | BOOKE_INTERRUPT_DOORBELL\n" - ">>=20\n" - ">> Those aren't the only exceptions that can end up going to the host. =20\n" + ">> \n" + ">> Those aren't the only exceptions that can end up going to the host. \n" ">> We could\n" ">> get a TLB miss that results in a heavyweight MMIO exit, etc.\n" - ">=20\n" - "> This is like host handler, so I'm just disabling soft interrupt =20\n" - "> during kvmppc_restart_interrupt() for Doorbell interrupt/Decrementer =20\n" + "> \n" + "> This is like host handler, so I'm just disabling soft interrupt \n" + "> during kvmppc_restart_interrupt() for Doorbell interrupt/Decrementer \n" "> Interrupt/External Input Interrupt.\n" - ">=20\n" - "> I don't see anything should be disabled for any TLB exception in host =20\n" + "> \n" + "> I don't see anything should be disabled for any TLB exception in host \n" "> handler.\n" "\n" - "Every exception type needs consistent lazy EE state once we hit the =20\n" + "Every exception type needs consistent lazy EE state once we hit the \n" "local_irq_enable() (or any other C code that cares).\n" "\n" - "Plus, if you're going to add code to make something conditional, you =20\n" - "should have a good reason for making it conditional. Being more like =20\n" - "the 64-bit host handler for its own sake isn't good enough, especially =20\n" + "Plus, if you're going to add code to make something conditional, you \n" + "should have a good reason for making it conditional. Being more like \n" + "the 64-bit host handler for its own sake isn't good enough, especially \n" "if you introduce differences between 32 and 64 bit in the process.\n" "\n" ">> And I'd rather see any fix for this problem stay out of the asm code.\n" - ">=20\n" - "> We already have an appropriate SOFT_DISABLE_INTS so I think we can =20\n" + "> \n" + "> We already have an appropriate SOFT_DISABLE_INTS so I think we can \n" "> take this easily :)\n" "\n" - "An unconditional call to hard_irq_disable() at the beginning of =20\n" - "kvmppc_handle_exit() should suffice. I already have this in the next =20\n" + "An unconditional call to hard_irq_disable() at the beginning of \n" + "kvmppc_handle_exit() should suffice. I already have this in the next \n" "version of my patch that I'll be posting shortly.\n" "\n" - "Note that we need this on 32-bit as well, so that trace_hardirqs_off() =20\n" + "Note that we need this on 32-bit as well, so that trace_hardirqs_off() \n" "gets called.\n" "\n" - -Scott= + -Scott -1426093a9a1b14778148ff5255b8df827403d7c8bc08bc940ef6918ac9bb32c8 +f91dc3e863604637017190e7ed65e0b8b3348e1af373bf475fe56c58ced0e3de
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.