Hi, Philippe, On Wed, Apr 29, 2020 at 5:18 PM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > On 4/29/20 3:52 AM, Huacai Chen wrote: > > Hi, Philippe and Aleksandar, > > > > I'm not refusing to change my patch, but I have two questions: > > 1, Why we should identify Loongson-3 to deliver IP3? It seems that > > deliver all IPs (IP2~IP7) unconditionally is harmless as well. > > 2, How to identify Loongson-3 by Config6/Config7? Loongson-3 is not > > the only processor which has Config6/Config7. > Please don't top-post on technical lists, it makes the conversation > harder to follow. > > This code is modelling the device, not KVM. > > Commit b1bd8b28cca is not very verbose. I wonder why not delivering all > IRQs to kvm_mips_set_interrupt, that would make this patch simpler. > > I think the problem in QEMU MIPS IRQ delivery is one implementation is > in cpu_mips_irq_request() while another one (vectored IRQ) in > cpu_mips_hw_interrupts_pending (see 138afb024bb) and KVM is also in the > middle. I think the previous code only deliver IP2 is because KVM/MIPS only use IP2 for external interrupts, but now I have changed KVM/MIPS as well, please see: https://patchwork.kernel.org/patch/11507591/
> > And I see you selected CP0C3_VInt in the R4 definition... so what is > delivered here? CP0C3_VInt just indicates the capability, kernel of Loongson-3 doesn't use VINT. > > > > > Huacai > > > > On Wed, Apr 29, 2020 at 2:58 AM Aleksandar Markovic > > <aleksandar.qemu.de...@gmail.com> wrote: > >> > >> уто, 28. апр 2020. у 10:21 chen huacai <zltjiang...@gmail.com> је > >> написао/ла: > >>> > >>> Hi, Philippe, > >>> > >>> On Mon, Apr 27, 2020 at 5:57 PM Philippe Mathieu-Daudé <f4...@amsat.org> > >>> wrote: > >>>> > >>>> On 4/27/20 11:33 AM, Huacai Chen wrote: > >>>>> Currently, KVM/MIPS only deliver I/O interrupt via IP2, this patch add > >>>>> IP2 delivery as well, because Loongson-3 based machine use both IRQ2 > > "IP3 delivery as well"? Sorry, this is my fault. > > >>>>> (CPU's IP2) and IRQ3 (CPU's IP3). > >>>>> > >>>>> Signed-off-by: Huacai Chen <che...@lemote.com> > >>>>> Co-developed-by: Jiaxun Yang <jiaxun.y...@flygoat.com> > >>>>> --- > >>>>> hw/mips/mips_int.c | 6 ++---- > >>>>> 1 file changed, 2 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c > >>>>> index 796730b..5526219 100644 > >>>>> --- a/hw/mips/mips_int.c > >>>>> +++ b/hw/mips/mips_int.c > >>>>> @@ -48,16 +48,14 @@ static void cpu_mips_irq_request(void *opaque, int > >>>>> irq, int level) > >>>>> if (level) { > >>>>> env->CP0_Cause |= 1 << (irq + CP0Ca_IP); > >>>>> > >>>>> - if (kvm_enabled() && irq == 2) { > >>>>> + if (kvm_enabled() && (irq == 2 || irq == 3)) > >>>> > >>>> Shouldn't we check env->CP0_Config6 (or Config7) has the required > >>>> feature first? > >>> I'm sorry that I can't understand IRQ delivery has something to do > >>> with Config6/Config7, to identify Loongson-3? > >>> > >> > >> Obviously, yes. > >> > >> Thanks, > >> Aleksandar > >> > >> > >>>> > >>>>> kvm_mips_set_interrupt(cpu, irq, level); > >>>>> - } > >>>>> > >>>>> } else { > >>>>> env->CP0_Cause &= ~(1 << (irq + CP0Ca_IP)); > >>>>> > >>>>> - if (kvm_enabled() && irq == 2) { > >>>>> + if (kvm_enabled() && (irq == 2 || irq == 3)) > >>>>> kvm_mips_set_interrupt(cpu, irq, level); > >>>>> - } > >>>>> } > >>>>> > >>>>> if (env->CP0_Cause & CP0Ca_IP_mask) { > >>>>> > >>> > >>> > >>> > >>> -- > >>> Huacai Chen > >