Skip to content
Snippets Groups Projects
  1. Sep 08, 2023
  2. Sep 07, 2023
    • Philippe Mathieu-Daudé's avatar
      sysemu/kvm: Restrict kvm_pc_setup_irq_routing() to x86 targets · bb781b94
      Philippe Mathieu-Daudé authored
      
      kvm_pc_setup_irq_routing() is only defined for x86 targets (in
      hw/i386/kvm/apic.c). Its declaration is pointless on all
      other targets.
      
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-ID: <20230904124325.79040-14-philmd@linaro.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      bb781b94
    • Philippe Mathieu-Daudé's avatar
      sysemu/kvm: Restrict kvm_has_pit_state2() to x86 targets · fc30abf8
      Philippe Mathieu-Daudé authored
      
      kvm_has_pit_state2() is only defined for x86 targets (in
      target/i386/kvm/kvm.c). Its declaration is pointless on
      all other targets. Have it return a boolean.
      
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-ID: <20230904124325.79040-13-philmd@linaro.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      fc30abf8
    • Philippe Mathieu-Daudé's avatar
      target/i386: Allow elision of kvm_hv_vpindex_settable() · ef1cf689
      Philippe Mathieu-Daudé authored
      
      Call kvm_enabled() before kvm_hv_vpindex_settable()
      to let the compiler elide its call.
      
      kvm-stub.c is now empty, remove it.
      
      Suggested-by: default avatarDaniel Henrique Barboza <dbarboza@ventanamicro.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-ID: <20230904124325.79040-9-philmd@linaro.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      ef1cf689
    • Philippe Mathieu-Daudé's avatar
      target/i386: Allow elision of kvm_enable_x2apic() · 9926cf34
      Philippe Mathieu-Daudé authored
      
      Call kvm_enabled() before kvm_enable_x2apic() to let the compiler elide
      its call.  Cleanup the code by simplifying "!xen_enabled() &&
      kvm_enabled()" to just "kvm_enabled()".
      
      Suggested-by: default avatarDaniel Henrique Barboza <dbarboza@ventanamicro.com>
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-ID: <20230904124325.79040-8-philmd@linaro.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      9926cf34
    • Philippe Mathieu-Daudé's avatar
      hw/i386/fw_cfg: Include missing 'cpu.h' header · 2686bbce
      Philippe Mathieu-Daudé authored
      
      fw_cfg_build_feature_control() uses CPUID_EXT_VMX which is
      defined in "target/i386/cpu.h".
      
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-ID: <20230904124325.79040-4-philmd@linaro.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      2686bbce
    • Philippe Mathieu-Daudé's avatar
      hw/i386/pc: Include missing 'cpu.h' header · d1aa2f50
      Philippe Mathieu-Daudé authored
      
      Both pc_piix.c and pc_q35.c files use CPU_VERSION_LEGACY
      which is defined in "target/i386/cpu.h".
      
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-ID: <20230904124325.79040-3-philmd@linaro.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      d1aa2f50
    • Philippe Mathieu-Daudé's avatar
      hw/i386/pc: Include missing 'sysemu/tcg.h' header · e44d989a
      Philippe Mathieu-Daudé authored
      
      Since commit 6f529b75 ("target/i386: move FERR handling
      to target/i386") pc_q35_init() calls tcg_enabled() which
      is declared in "sysemu/tcg.h".
      
      Signed-off-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-ID: <20230904124325.79040-2-philmd@linaro.org>
      Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      e44d989a
    • Niklas Cassel's avatar
      hw/ide/ahci: fix broken SError handling · 9f894235
      Niklas Cassel authored
      
      When encountering an NCQ error, you should not write the NCQ tag to the
      SError register. This is completely wrong.
      
      The SError register has a clear definition, where each bit represents a
      different error, see PxSERR definition in AHCI 1.3.1.
      
      If we write a random value (like the NCQ tag) in SError, e.g. Linux will
      read SError, and will trigger arbitrary error handling depending on the
      NCQ tag that happened to be executing.
      
      In case of success, ncq_cb() will call ncq_finish().
      In case of error, ncq_cb() will call ncq_err() (which will clear
      ncq_tfs->used), and then call ncq_finish(), thus using ncq_tfs->used is
      sufficient to tell if finished should get set or not.
      
      Signed-off-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-id: 20230609140844.202795-9-nks@flawful.org
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      9f894235
    • Niklas Cassel's avatar
      hw/ide/ahci: fix ahci_write_fis_sdb() · 7e85cb0d
      Niklas Cassel authored
      
      When there is an error, we need to raise a TFES error irq, see AHCI 1.3.1,
      5.3.13.1 SDB:Entry.
      
      If ERR_STAT is set, we jump to state ERR:FatalTaskfile, which will raise
      a TFES IRQ unconditionally, regardless if the I bit is set in the FIS or
      not.
      
      Thus, we should never raise a normal IRQ after having sent an error IRQ.
      
      It is valid to signal successfully completed commands as finished in the
      same SDB FIS that generates the error IRQ. The important thing is that
      commands that did not complete successfully (e.g. commands that were
      aborted, do not get the finished bit set).
      
      Before this commit, there was never a TFES IRQ raised on NCQ error.
      
      Signed-off-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-id: 20230609140844.202795-8-nks@flawful.org
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      7e85cb0d
    • Niklas Cassel's avatar
      hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set · 1a16ce64
      Niklas Cassel authored
      
      For NCQ, PxCI is cleared on command queued successfully.
      For non-NCQ, PxCI is cleared on command completed successfully.
      Successfully means ERR_STAT, BUSY and DRQ are all cleared.
      
      A command that has ERR_STAT set, does not get to clear PxCI.
      See AHCI 1.3.1, section 5.3.8, states RegFIS:Entry and RegFIS:ClearCI,
      and 5.3.16.5 ERR:FatalTaskfile.
      
      In the case of non-NCQ commands, not clearing PxCI is needed in order
      for host software to be able to see which command slot that failed.
      
      Signed-off-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
      Message-id: 20230609140844.202795-7-nks@flawful.org
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      1a16ce64
    • Niklas Cassel's avatar
      hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared · d73b84d0
      Niklas Cassel authored
      
      According to AHCI 1.3.1 definition of PxSACT:
      This field is cleared when PxCMD.ST is written from a '1' to a '0' by
      software. This field is not cleared by a COMRESET or a software reset.
      
      According to AHCI 1.3.1 definition of PxCI:
      This field is also cleared when PxCMD.ST is written from a '1' to a '0'
      by software.
      
      Clearing PxCMD.ST is part of the error recovery procedure, see
      AHCI 1.3.1, section "6.2 Error Recovery".
      
      If we don't clear PxCI on error recovery, the previous command will
      incorrectly still be marked as pending after error recovery.
      
      Signed-off-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-id: 20230609140844.202795-6-nks@flawful.org
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      d73b84d0
    • Niklas Cassel's avatar
      hw/ide/ahci: simplify and document PxCI handling · e2a5d9b3
      Niklas Cassel authored
      
      The AHCI spec states that:
      For NCQ, PxCI is cleared on command queued successfully.
      
      For non-NCQ, PxCI is cleared on command completed successfully.
      (A non-NCQ command that completes with error does not clear PxCI.)
      
      The current QEMU implementation either clears PxCI in check_cmd(),
      or in ahci_cmd_done().
      
      check_cmd() will clear PxCI for a command if handle_cmd() returns 0.
      handle_cmd() will return -1 if BUSY or DRQ is set.
      
      The QEMU implementation for NCQ commands will currently not set BUSY
      or DRQ, so they will always have PxCI cleared by handle_cmd().
      ahci_cmd_done() will never even get called for NCQ commands.
      
      Non-NCQ commands are executed by ide_bus_exec_cmd().
      Non-NCQ commands in QEMU are implemented either in a sync or in an async
      way.
      
      For non-NCQ commands implemented in a sync way, the command handler will
      return true, and when ide_bus_exec_cmd() sees that a command handler
      returns true, it will call ide_cmd_done() (which will call
      ahci_cmd_done()). For a command implemented in a sync way,
      ahci_cmd_done() will do nothing (since busy_slot is not set). Instead,
      after ide_bus_exec_cmd() has finished, check_cmd() will clear PxCI for
      these commands.
      
      For non-NCQ commands implemented in an async way (using either aiocb or
      pio_aiocb), the command handler will return false, ide_bus_exec_cmd()
      will not call ide_cmd_done(), instead it is expected that the async
      callback function will call ide_cmd_done() once the async command is
      done. handle_cmd() will set busy_slot, if and only if BUSY or DRQ is
      set, and this is checked _after_ ide_bus_exec_cmd() has returned.
      handle_cmd() will return -1, so check_cmd() will not clear PxCI.
      When the async callback calls ide_cmd_done() (which will call
      ahci_cmd_done()), it will see that busy_slot is set, and
      ahci_cmd_done() will clear PxCI.
      
      This seems racy, since busy_slot is set _after_ ide_bus_exec_cmd() has
      returned. The callback might come before busy_slot gets set. And it is
      quite confusing that ahci_cmd_done() will be called for all non-NCQ
      commands when the command is done, but will only clear PxCI in certain
      cases, even though it will always write a D2H FIS and raise an IRQ.
      
      Even worse, in the case where ahci_cmd_done() does not clear PxCI, it
      still raises an IRQ. Host software might thus read an old PxCI value,
      since PxCI is cleared (by check_cmd()) after the IRQ has been raised.
      
      Try to simplify this by always setting busy_slot for non-NCQ commands,
      such that ahci_cmd_done() will always be responsible for clearing PxCI
      for non-NCQ commands.
      
      For NCQ commands, clear PxCI when we receive the D2H FIS, but before
      raising the IRQ, see AHCI 1.3.1, section 5.3.8, states RegFIS:Entry and
      RegFIS:ClearCI.
      
      Signed-off-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
      Message-id: 20230609140844.202795-5-nks@flawful.org
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      e2a5d9b3
    • Niklas Cassel's avatar
      hw/ide/ahci: write D2H FIS when processing NCQ command · 2967dc82
      Niklas Cassel authored
      
      The way that BUSY + PxCI is cleared for NCQ (FPDMA QUEUED) commands is
      described in SATA 3.5a Gold:
      
      11.15 FPDMA QUEUED command protocol
      DFPDMAQ2: ClearInterfaceBsy
      "Transmit Register Device to Host FIS with the BSY bit cleared to zero
      and the DRQ bit cleared to zero and Interrupt bit cleared to zero to
      mark interface ready for the next command."
      
      PxCI is currently cleared by handle_cmd(), but we don't write the D2H
      FIS to the FIS Receive Area that actually caused PxCI to be cleared.
      
      Similar to how ahci_pio_transfer() calls ahci_write_fis_pio() with an
      additional parameter to write a PIO Setup FIS without raising an IRQ,
      add a parameter to ahci_write_fis_d2h() so that ahci_write_fis_d2h()
      also can write the FIS to the FIS Receive Area without raising an IRQ.
      
      Change process_ncq_command() to call ahci_write_fis_d2h() without
      raising an IRQ (similar to ahci_pio_transfer()), such that the FIS
      Receive Area is in sync with the PxTFD shadow register.
      
      E.g. Linux reads status and error fields from the FIS Receive Area
      directly, so it is wise to keep the FIS Receive Area and the PxTFD
      shadow register in sync.
      
      Signed-off-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
      Message-id: 20230609140844.202795-4-nks@flawful.org
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      2967dc82
    • Niklas Cassel's avatar
      hw/ide/core: set ERR_STAT in unsupported command completion · c3461c62
      Niklas Cassel authored
      
      Currently, the first time sending an unsupported command
      (e.g. READ LOG DMA EXT) will not have ERR_STAT set in the completion.
      Sending the unsupported command again, will correctly have ERR_STAT set.
      
      When ide_cmd_permitted() returns false, it calls ide_abort_command().
      ide_abort_command() first calls ide_transfer_stop(), which will call
      ide_transfer_halt() and ide_cmd_done(), after that ide_abort_command()
      sets ERR_STAT in status.
      
      ide_cmd_done() for AHCI will call ahci_write_fis_d2h() which writes the
      current status in the FIS, and raises an IRQ. (The status here will not
      have ERR_STAT set!).
      
      Thus, we cannot call ide_transfer_stop() before setting ERR_STAT, as
      ide_transfer_stop() will result in the FIS being written and an IRQ
      being raised.
      
      The reason why it works the second time, is that ERR_STAT will still
      be set from the previous command, so when writing the FIS, the
      completion will correctly have ERR_STAT set.
      
      Set ERR_STAT before writing the FIS (calling cmd_done), so that we will
      raise an error IRQ correctly when receiving an unsupported command.
      
      Signed-off-by: default avatarNiklas Cassel <niklas.cassel@wdc.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Message-id: 20230609140844.202795-3-nks@flawful.org
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      c3461c62
  3. Sep 06, 2023
  4. Sep 01, 2023
Loading