Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

netkvm: does it need to wait when StopFlows #1221

Open
wangyan0507 opened this issue Dec 16, 2024 · 2 comments
Open

netkvm: does it need to wait when StopFlows #1221

wangyan0507 opened this issue Dec 16, 2024 · 2 comments
Assignees
Labels

Comments

@wangyan0507
Copy link

When shutdown the windows vm with netkvm driver, shoud it wait the m_Counter to zero for m_RxStateMachine?
The shutdown stack:

ndis!ndisDispatchIoWorkItem
->netkvm!OnSetPowerWorkItem
  ->netkvm!ParaNdis_PowerOff
    ->netkvm!CMiniportStateMachine::NotifySuspended
      ->netkvm!CMiniportStateMachine::StopFlows
        ->netkvm!CNdisList<CDataFlowStateMachine,CRawAccess,CNonCountingObject>::ForEach
          ->netkvm!CFlowStateMachine::Stop
            ->m_NoOutstandingItems.Wait();  // which will wait the m_Counter to become StoppedMask.

For shutdown vm scene, when there is a lot of incomping packets, the m_Counter will be released as follows for m_RxStateMachine:

ParaNdis6_ReturnNetBufferLists()
  ->pContext->m_RxStateMachine.UnregisterOutstandingItems(nofNetkvm);  // which will release for m_Counter.
    ->m_Counter.Release(NumItems)
    -> CheckCompletion(value)
      ->CompleteStopping()
        ->m_NoOutstandingItems.Notify(); // which will awake the upper 'm_NoOutstandingItems.Wait()'.

But from the microsoft doc for OID_PNP_SET_POWER:
The miniport driver that supports NDIS 6.30 and later versions of NDIS must also do the following:
Not wait for the completion of pending receive indications through calls to its MiniportReturnNetBufferLists function.

Thanks in advance!
Yan Wang

@YanVugenfirer
Copy link
Collaborator

@wangyan0507 Unfortunately, paravirtualizaed "HW" behaves differently than actual HW in this case, so we must wait.
If we release the memory (the NET_BUFFER_LISTs), the host might still continue to write to memory that can potentially be reused by the guest.

@wangyan0507
Copy link
Author

wangyan0507 commented Dec 16, 2024

@YanVugenfirer Sorry, I am not understand very well.
The shutdown process:

ndis!ndisDispatchIoWorkItem
->netkvm!OnSetPowerWorkItem
  ->netkvm!ParaNdis_PowerOff
    ->netkvm!CMiniportStateMachine::NotifySuspended
      ->netkvm!CMiniportStateMachine::StopFlows
        ->netkvm!CNdisList<CDataFlowStateMachine,CRawAccess,CNonCountingObject>::ForEach
          ->netkvm!CFlowStateMachine::Stop
            ->m_Counter.SetMask(StoppedMask); // Or with StoppedMask to m_Counter.
            ->m_NoOutstandingItems.Wait();    // Wait the m_Counter to become StoppedMask.

The normal rx process:

ndis!ndisMiniportDpc
  ->netkvm!MiniportMSIInterruptDpc
    ->netkvm!ParaNdis_RXTXDPCWorkBody
      ->netkvm!RxDPCWorkBody
        ->pContext->m_RxStateMachine.RegisterOutstandingItems // Add reference for m_Counter.
        ->ndis!NdisMIndicateReceiveNetBufferLists
          ->...
            ->netkvm!ParaNdis6_ReturnNetBufferLists
              ->pContext->m_RxStateMachine.UnregisterOutstandingItems
                ->m_Counter.Release(NumItems) // Release NumItems for m_Counter.
                -> CheckCompletion(value)
                  ->CompleteStopping()
                    ->m_NoOutstandingItems.Notify(); // Wake the upper 'm_NoOutstandingItems.Wait()'.

In "Not wait for the completion of pending receive indications through calls to its MiniportReturnNetBufferLists function.", does it mean no need to wait the call for netkvm!ParaNdis6_ReturnNetBufferLists? If it is, the call m_NoOutstandingItems.Wait() may be get stuck, because it depends on the m_Counter to become StoppedMask.
You mentioned "If we release the memory (the NET_BUFFER_LISTs)", I don't know where do we release the memory?

Thanks!
Yan Wang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants