Re: unwind(8): refactor & simplify refcounting

2019-11-13 Thread Otto Moerbeek
On Tue, Nov 12, 2019 at 05:45:38PM +0100, Florian Obser wrote:

> Did I get this right? I'd appreciate it if someone could give this a
> once over.
> 
> Since resolve() switched to a callback mechanism all uw_resolver objects
> pass through resolve() and either asr_resolve_done() or
> ub_resolve_done().
> With that we can pull resolver_ref() and resolver_unref() into those
> functions to make the reference counting easier.
> Only check_resolver is special since it needs to refcount the to be
> checked resolver. But the resolver doing the actual work is
> automatically refcounted by resolve() and *_resolve_done().
> One last piece of the puzzle is to track the uw_resolver object in
> cb_data so that the *_resolve_done() functions have access to it.
> This also allowes us to remove the ad-hoc passing of the resolver in
> query_imsg. Since the callback functions all need access to the
> resolver that did the work we pass it in as first argument.
> 

Reviewed the code, did some tests and it all looks good.

One nit: I would have declared a typedef for the callback funtion type
to be used in the struct resolver_cb_data and the prototype and the
definition of resolve(), it makes those lines easier to read.  But ok
anyway,

-Otto


> diff --git resolver.c resolver.c
> index d1dce2dec71..2b7d81d29fc 100644
> --- resolver.c
> +++ resolver.c
> @@ -92,14 +92,11 @@ struct uw_resolver {
>   int64_t  histogram[nitems(histogram_limits)];
>  };
>  
> -struct check_resolver_data {
> - struct uw_resolver  *res;
> - struct uw_resolver  *check_res;
> -};
> -
>  struct resolver_cb_data {
> - void(*cb)(void *, int, void *, int, int, char *);
> - void*data;
> + void(*cb)(struct uw_resolver *, void *, int, void *,
> + int, int, char *);
> + void*data;
> + struct uw_resolver  *res;
>  };
>  
>  __dead void   resolver_shutdown(void);
> @@ -108,9 +105,10 @@ void  
> resolver_dispatch_frontend(int, short, void *);
>  void  resolver_dispatch_captiveportal(int, short, void *);
>  void  resolver_dispatch_main(int, short, void *);
>  int   resolve(struct uw_resolver *, const char*, int, int,
> -  void*, void (*cb)(void *, int, void *, int, int,
> -  char *));
> -void  resolve_done(void *, int, void *, int, int, char *);
> +  void*, void (*cb)(struct uw_resolver *, void *,
> +  int, void *, int, int, char *));
> +void  resolve_done(struct uw_resolver *, void *, int, void *,
> +  int, int, char *);
>  void  ub_resolve_done(void *, int, void *, int, int, char *,
>int);
>  void  asr_resolve_done(struct asr_result *, void *);
> @@ -129,8 +127,8 @@ void   set_forwarders_oppdot(struct 
> uw_resolver *,
>  void  resolver_check_timo(int, short, void *);
>  void  resolver_free_timo(int, short, void *);
>  void  check_resolver(struct uw_resolver *);
> -void  check_resolver_done(void *, int, void *, int, int,
> -  char *);
> +void  check_resolver_done(struct uw_resolver *, void *, int,
> +  void *, int, int, char *);
>  void  schedule_recheck_all_resolvers(void);
>  int   check_forwarders_changed(struct uw_forwarder_head *,
>struct uw_forwarder_head *);
> @@ -154,8 +152,8 @@ int
> check_captive_portal_changed(struct uw_conf *,
>struct uw_conf *);
>  void  trust_anchor_resolve(void);
>  void  trust_anchor_timo(int, short, void *);
> -void  trust_anchor_resolve_done(void *, int, void *, int,
> -  int, char *);
> +void  trust_anchor_resolve_done(struct uw_resolver *, void *,
> +  int, void *, int, int, char *);
>  void  add_autoconf_forwarders(struct imsg_rdns_proposal *);
>  void  rem_autoconf_forwarders(struct imsg_rdns_proposal *);
>  struct uw_forwarder  *find_forwarder(struct uw_forwarder_head *,
> @@ -480,14 +478,10 @@ resolver_dispatch_frontend(int fd, short event, void 
> *bula)
>   log_debug("%s: choosing %s", __func__,
>   uw_resolver_type_str[res->type]);
>  
> - query_imsg->resolver = res;
> - resolver_ref(res);
> -
>   clock_gettime(CLOCK_MONOTONIC, _imsg->tp);
>  
> - if ((resolve(res, query_imsg->qname, query_imsg->t,
> -

Re: Remove NetChip from cdce

2019-11-13 Thread Patrick Wildt
On Wed, Nov 13, 2019 at 06:40:00PM -0700, Aaron Bieber wrote:
> Hi,
> 
> I have a raspberry pi 0 that attaches as:
>   cdce0 at uhub0 port 3 configuration 2 interface 0 "Linux 4.19.75+ with \
> 2098.usb RNDIS/Ethernet Gadget" rev 2.00/4.19 addr 10
> 
> Unfortunately the cdce interface does not work with this particular device.
> 
> On the linux end, it turns out that cdc and rndis are being presented. When
> NetChip is removed from the cdce driver, urndis(4) picks up the slack and
> things work as expected! Linux also has the product id defined as RNDIS.

The thing is that Linux provides multiple "configurations" (as in:
urndis(4), cdce(4), ...) over the same device.  Hardcoding this
combination of vendor/product id will make cdce(4) always attach,
even though the configuration is actually urndis(4).  In this case
Linux improved well enough so that our match function checking for
class/subclass should be the right thing to do.

Thus, ok patrick@

> Debugged with patrick@ who is awesome and I now owe a beer or two for his
> help! :D
> 
> OK?
> 
> Cheers,
> Aaron
> 
> diff --git a/sys/dev/usb/if_cdce.c b/sys/dev/usb/if_cdce.c
> index abf0521ccd6..816bdc29c10 100644
> --- a/sys/dev/usb/if_cdce.c
> +++ b/sys/dev/usb/if_cdce.c
> @@ -100,7 +100,6 @@ const struct cdce_type cdce_devs[] = {
>  {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN }, CDCE_CRC32 },
>  {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN2 }, CDCE_CRC32 },
>  {{ USB_VENDOR_GMATE, USB_PRODUCT_GMATE_YP3X00 }, 0 },
> -{{ USB_VENDOR_NETCHIP, USB_PRODUCT_NETCHIP_ETHERNETGADGET }, 0 },
>  {{ USB_VENDOR_COMPAQ, USB_PRODUCT_COMPAQ_IPAQLINUX }, 0 },
>  {{ USB_VENDOR_AMBIT, USB_PRODUCT_AMBIT_NTL_250 }, CDCE_SWAPUNION },
>  };
> 
> -- 
> PGP: 0x1F81112D62A9ADCE / 3586 3350 BFEA C101 DB1A  4AF0 1F81 112D 62A9 ADCE
> 



Remove NetChip from cdce

2019-11-13 Thread Aaron Bieber
Hi,

I have a raspberry pi 0 that attaches as:
  cdce0 at uhub0 port 3 configuration 2 interface 0 "Linux 4.19.75+ with \
2098.usb RNDIS/Ethernet Gadget" rev 2.00/4.19 addr 10

Unfortunately the cdce interface does not work with this particular device.

On the linux end, it turns out that cdc and rndis are being presented. When
NetChip is removed from the cdce driver, urndis(4) picks up the slack and
things work as expected! Linux also has the product id defined as RNDIS.

Debugged with patrick@ who is awesome and I now owe a beer or two for his
help! :D

OK?

Cheers,
Aaron

diff --git a/sys/dev/usb/if_cdce.c b/sys/dev/usb/if_cdce.c
index abf0521ccd6..816bdc29c10 100644
--- a/sys/dev/usb/if_cdce.c
+++ b/sys/dev/usb/if_cdce.c
@@ -100,7 +100,6 @@ const struct cdce_type cdce_devs[] = {
 {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN }, CDCE_CRC32 },
 {{ USB_VENDOR_MOTOROLA2, USB_PRODUCT_MOTOROLA2_USBLAN2 }, CDCE_CRC32 },
 {{ USB_VENDOR_GMATE, USB_PRODUCT_GMATE_YP3X00 }, 0 },
-{{ USB_VENDOR_NETCHIP, USB_PRODUCT_NETCHIP_ETHERNETGADGET }, 0 },
 {{ USB_VENDOR_COMPAQ, USB_PRODUCT_COMPAQ_IPAQLINUX }, 0 },
 {{ USB_VENDOR_AMBIT, USB_PRODUCT_AMBIT_NTL_250 }, CDCE_SWAPUNION },
 };

-- 
PGP: 0x1F81112D62A9ADCE / 3586 3350 BFEA C101 DB1A  4AF0 1F81 112D 62A9 ADCE



Re: ix(4) need support for X553

2019-11-13 Thread Stuart Henderson
Apart from the obvious style(9) problems, can anyone give me guidance on
how to approach this diff? I'm quite uneasy at the amount of changes but these
nics are quite important, they're all over the place on Atom C3xxx boards
which are pretty much the main "next step up" from APUs.

I have an A2SDi-2C-HLN4F that I need to put in service soon (not so
bothered about the ix on this box, I'm using 4xSFP+ ixl, but giving it
a spin) - applying my merged diff (https://junkpile.org/ixgbe.diff2)
and adding PCI_PRODUCT_INTEL_X550EM_A_1G_T_L things work so far in a
quick test.

- MACs look sane

- no problems seen in rx/tx at 1G link speed, I haven't tried other
speeds; looking at Linux diffs it seems it may need a little change for 10M

- haven't tried jumbos yet

- seems fairly reliable so far but tx performance is a bit poor, ~500M
max sends from tcpbench - rx speeds are fine

- NFS works

- dmesg/pcidevs below in case they're of interest


On 2019/10/30 21:43, Stuart Henderson wrote:
> On 2019/10/30 07:34, sven falempin wrote:
> > https://github.com/dohnuts/wip/blob/master/ixgbe.diff
> > 
> > Needs lots of cleaning
> > 
> > On Wed, Oct 30, 2019 at 6:59 AM Joerg Goltermann  wrote:
> > 
> > > Hello,
> > >
> > > I have a new Lanner NCA-1510A (with a Intel C3558) which has some
> > > X553 SGMII ethernet ports. Unfortunately there is no support in ix(4)
> > > for this type.
> > >
> > > Is anyone working on an update for the ix(4) to support the new
> > > hardware?
> > >
> > > I took a look at the actual ix(4) and it's a bit confusing. I'm
> > > not sure about the "version" of this driver compared to FreeBSD/
> > > Intel version.
> > >
> > > Wouldn't it be nice to have a more "stock" version of the driver. This
> > > would make it much easier to port updates from FreeBSD/Intel to OpenBSD.
> > >
> > > Any feedback is appreciated.
> > >
> > >   - Joerg
> > >
> 
> Oh I've got a machine coming soon and I have a nasty feeling it's going to
> have one of those ..
> 
> That diff is stale, I've merged conflicts at https://junkpile.org/ixgbe.diff2,
> and it builds but totally untested.
> 

$  sysctl hw
hw.machine=amd64
hw.model=Intel(R) Atom(TM) CPU C3338 @ 1.50GHz
hw.ncpu=2
hw.byteorder=1234
hw.pagesize=4096
hw.disknames=sd0:f232b1bd76ed931b
hw.diskcount=1
hw.sensors.cpu0.temp0=32.00 degC
hw.cpuspeed=2200
hw.setperf=199
hw.vendor=Supermicro
hw.product=Super Server
hw.version=0123456789
hw.serialno=0123456789
hw.uuid=----3cecef004fd4
hw.physmem=8530952192
hw.usermem=8530939904
hw.ncpufound=2
hw.allowpowerdown=1
hw.perfpolicy=manual
hw.smt=0
hw.ncpuonline=2


OpenBSD 6.6-current (GENERIC.MP) #1: Wed Nov 13 23:40:03 UTC 2019
sthen@xxx:/sys/arch/amd64/compile/GENERIC.MP
real mem = 8530952192 (8135MB)
avail mem = 8260022272 (7877MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7f0c3000 (32 entries)
bios0: vendor American Megatrends Inc. version "1.1c" date 06/25/2019
bios0: Supermicro Super Server
acpi0 at bios0: ACPI 6.1
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP FPDT FIDT SPMI MCFG WDAT APIC BDAT HPET UEFI SSDT DMAR 
HEST BERT ERST EINJ WSMT
acpi0: wakeup devices XHC1(S4) OBL1(S4) LAN1(S4) PEX0(S4) LAN2(S4) LAN3(S4) 
PEX1(S4) PEX6(S4) PEX7(S4)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xe000, bus 0-255
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 12 (boot processor)
cpu0: Intel(R) Atom(TM) CPU C3338 @ 1.50GHz, 2200.42 MHz, 06-5f-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,CX16,xTPR,PDCM,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SMEP,ERMS,MPX,RDSEED,SMAP,CLFLUSHOPT,PT,SHA,MD_CLEAR,IBRS,IBPB,STIBP,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 2MB 64b/line 16-way L2 cache
cpu0: cannot disable silicon debug
cpu0: smt 0, core 6, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 25MHz
cpu0: mwait min=64, max=64, C-substates=0.2.0.2, IBE
cpu1 at mainbus0: apid 24 (application processor)
cpu1: Intel(R) Atom(TM) CPU C3338 @ 1.50GHz, 2200.02 MHz, 06-5f-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,CX16,xTPR,PDCM,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,SMEP,ERMS,MPX,RDSEED,SMAP,CLFLUSHOPT,PT,SHA,MD_CLEAR,IBRS,IBPB,STIBP,SSBD,SENSOR,ARAT,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 2MB 64b/line 16-way L2 cache
cpu1: cannot disable silicon debug
cpu1: smt 0, core 12, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins
acpihpet0 at acpi0: 2399 Hz
acpiprt0 at acpi0: bus 0 

Re: sysupgrade: Allow to use another directory for the sets

2019-11-13 Thread Renaud Allard



On 12/11/2019 19:02, Renaud Allard wrote:



On 12/11/2019 08:29, Theo de Raadt wrote:


Renaud, please test it for me like this:

  sysupgrade -d /

This interface is dangerously incorrect.



What about this one?


ping.

I haven't seen any reply on the prefix based patch, but what about also 
making -k (Keep the files in /home/_sysupgrade) implicit in case -d is used?




smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH: 1/3] MMIO handler in vmm(4)

2019-11-13 Thread Mike Larkin
On Sat, Nov 02, 2019 at 06:40:52AM +0900, Iori YONEJI wrote:
> On Tue, Oct 29, 2019 at 02:17:28AM -0700, Mike Larkin wrote:
> > On Thu, Oct 24, 2019 at 08:54:58AM +0900, Iori YONEJI wrote:
> > > Hello tech@,
> > > 
> > > Here is the patch discussed in the previous email. This part mainly
> > > covers changes in the declaration part and fault handlers.
> > > 
> > 
> > Hello,
> > 
> >  I read through the three diffs and have some feedback.
> > 
> > First, please reformat all 3 diffs using style(9) guidelines. There are many
> > spaces vs tabs issues in the diffs.
> Thank you for your review. I'll fix the issues on this weekend.
> 
> First of all, the reason of the spaces vs. tabs issues was due to
> mangling by the mailer. I fixed this now, but I'm not aware of other
> types of style issues, even I will go through the patches to find other
> style mismatches after started working on them.
> 
> > Second, there seems to be quite a bit of important code missing here:
> Most of them convinced me, but let me leave a few comment for them.
> 

Iori Yoneji and I are discussing this in another thread, but for the benefit
of searchability/posterity, see below for a summary.

> 1. Segment base and limit check
> > 1. There appears to be no handling of segment base and limits for 32 bit
> >instructions. These need to be read from the gdt and strictly adhered
> >to. For 64 bit mode, it's not as important, unless the guest has enabled
> >LMSLE (long mode segment limit enable) on AMD CPUs, in which case the
> >limits need to be checked also. If I recall correctly, there are also
> >some different rules that need to be followed for 32 bit segment use
> >relating to permission checks and segment types.
> I wasn't aware of limit check, but I think vcpu would be triggered #GP
> if the segment was out of range instead of EPT violation, wouldn't it?
>

The SDM seems to be a bit vague here about the rules for checking segment limits
in VMX non-root mode (eg, inside a VM) and whether that causes an exit or not
based on various configurations, specifically around segment limits. This is
likely an issue only for 32 bit guests (we can block LMSLE).

> 2. Privilege level check
> > 2. For that matter, there appears to be no handling of any permission or
> >privilege checks in the instruction emulator. This means any privilege
> >level can read or write any memory in the VM.
> Sorry, it is my very big fault. I must check CPU mode in next post.
> 

In the generic EPT case (not a segment limit issue), this is likely handled
properly already. We are discussing if it makes sense to put the permission
checks in anyway, in case this code ends up being used in other places.

> 3-4. 'A'ccessed and 'D'irty bit management
> > 3. There appears to be no handling of the updating of the 'A'ccessed or
> >'D'irty bits on successful page table walk and writing to a page
> >computed by that translation. Granted, this probably needs to go into
> >the existing translate_gva function for the 'A' bit, but the 'D' bit
> >needs to also be handled here.
> > 
> > 4. There appears to be no handling of the updating of the 'A' bit in the
> >GDT for the segment descriptor in use.
> Yes, but I'm not sure what would be the expected use of A/D bit
> management on MMIO region because it wouldn't be subject to swap in/out.
> I think I will understand the expected behavior after working on it,
> however, I couldn't get how it compromise the virtual machine features.
> 

There are certain degenerate cases that might require this, and we are unclear
under these circumstances if the CPU does this for us or if the emulator
needs to. As pointed out above, the need for A/D bits in this particular case
is likely minimal, but in the same theme as above, we may want to make the
code work generically instead of assuming the emulator will only ever be
used for accesses in the MMIO region.

-ml


> 5-7. Prefixes
> > 5. The code seems to ignore any segment prefix override bytes.
> > 
> > 6. The code seems to ignore forbidden instruction prefixes (like rex.w
> >or lock prefixes being placed before instructions where they don't
> >make sense or are prohibited by the SDM)
> > 
> > 7. For that matter, operand size encoding prefixes don't seem to be
> >honoured at all
> Yes, there are some prefixes I missed in the patches. I will enhance
> them to handle those prefixes.
> One thing to confirm: Is it OK not to calculate in the segment registers
> (whether override-ed or not) but just count the bytes, because we know
> the address the instruction intended to access?
> 
> 8. %rflags
> > 8. %rflags is not being updated in any scenario. It looks like there were
> >a few #defines for things like VMM_EMUL_ADD/SUB, which would require
> >setting various flags in %rflags, but I can't seem to find the code that
> >actually does these instructions, so maybe these are just old #defines?
> MOV is the only instruction in my