From 1182090a3eaeac27d476436c3588f58e5564f61f Mon Sep 17 00:00:00 2001
From: Daniel Verkamp <dverkamp@chromium.org>
Date: Wed, 13 May 2020 23:27:28 -0700
Subject: [PATCH] devices: usb: remove interrupter pending variable

The pending variable was just duplicating state that can easily be
determined based on whether the ring is empty.  Remove the variable and
replace it with an equivalent check in interrupt_if_needed() to avoid
the possibility of accidentally getting these out of sync.

BUG=chromium:1082930
TEST=cargo test -p devices

Change-Id: Icb90e3d09c43de244f5fecffb0e55d4635be6d2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2202744
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
---
 devices/src/usb/xhci/event_ring.rs  | 5 -----
 devices/src/usb/xhci/interrupter.rs | 9 +--------
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/devices/src/usb/xhci/event_ring.rs b/devices/src/usb/xhci/event_ring.rs
index 1742c777d8..4711c46bc7 100644
--- a/devices/src/usb/xhci/event_ring.rs
+++ b/devices/src/usb/xhci/event_ring.rs
@@ -144,11 +144,6 @@ impl EventRing {
         self.dequeue_pointer = addr;
     }
 
-    /// Get the enqueue pointer.
-    pub fn get_enqueue_pointer(&self) -> GuestAddress {
-        self.enqueue_pointer
-    }
-
     /// Check if event ring is empty.
     pub fn is_empty(&self) -> bool {
         self.enqueue_pointer == self.dequeue_pointer
diff --git a/devices/src/usb/xhci/interrupter.rs b/devices/src/usb/xhci/interrupter.rs
index c58ae599b8..6366050449 100644
--- a/devices/src/usb/xhci/interrupter.rs
+++ b/devices/src/usb/xhci/interrupter.rs
@@ -45,7 +45,6 @@ pub struct Interrupter {
     erdp: Register<u64>,
     event_handler_busy: bool,
     enabled: bool,
-    pending: bool,
     moderation_interval: u16,
     moderation_counter: u16,
     event_ring: EventRing,
@@ -61,7 +60,6 @@ impl Interrupter {
             erdp: regs.erdp.clone(),
             event_handler_busy: false,
             enabled: false,
-            pending: false,
             moderation_interval: 0,
             moderation_counter: 0,
             event_ring: EventRing::new(mem),
@@ -76,7 +74,6 @@ impl Interrupter {
     /// Add event to event ring.
     fn add_event(&mut self, trb: Trb) -> Result<()> {
         self.event_ring.add_event(trb).map_err(Error::AddEvent)?;
-        self.pending = true;
         self.interrupt_if_needed()
     }
 
@@ -169,9 +166,6 @@ impl Interrupter {
     pub fn set_event_ring_dequeue_pointer(&mut self, addr: GuestAddress) -> Result<()> {
         usb_debug!("interrupter set dequeue ptr addr {:#x}", addr.0);
         self.event_ring.set_dequeue_pointer(addr);
-        if addr == self.event_ring.get_enqueue_pointer() {
-            self.pending = false;
-        }
         self.interrupt_if_needed()
     }
 
@@ -186,7 +180,6 @@ impl Interrupter {
     pub fn interrupt(&mut self) -> Result<()> {
         usb_debug!("sending interrupt");
         self.event_handler_busy = true;
-        self.pending = false;
         self.usbsts.set_bits(USB_STS_EVENT_INTERRUPT);
         self.iman.set_bits(IMAN_INTERRUPT_PENDING);
         self.erdp.set_bits(ERDP_EVENT_HANDLER_BUSY);
@@ -194,7 +187,7 @@ impl Interrupter {
     }
 
     fn interrupt_if_needed(&mut self) -> Result<()> {
-        if self.enabled && self.pending && !self.event_handler_busy {
+        if self.enabled && !self.event_ring.is_empty() && !self.event_handler_busy {
             self.interrupt()?;
         }
         Ok(())